drill-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Daniel Barclay (Drill) (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (DRILL-3730) Change JDBC driver's DrillConnectionConfig to interface
Date Mon, 26 Oct 2015 17:38:27 GMT

    [ https://issues.apache.org/jira/browse/DRILL-3730?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14726115#comment-14726115
] 

Daniel Barclay (Drill) edited comment on DRILL-3730 at 10/26/15 5:37 PM:
-------------------------------------------------------------------------

> I would rather we did not change things that are not broken.

[edit:  I probably should have started off by mentioning that I submitted this JIRA report
then only because a reviewer pointed out a pre-existing (I think) TODO comment that didn't
have a DRILL-nnnn annotation and wanted me to file a report and add that before he approved
the commit.]


I'm trying to fix something that arguably _is_ broken:

{{TracingProxyDriver}} can trace calls across the rest of Drill's JDBC interface because it
can create proxy classes wrapping Drill's implementation classes, since they are defined in
terms of interfaces (e.g., JDBC's interface {{java.sql.Connection}} and Drill's derived interface
{{org.apache.drill.jdbc.DrillConnection}}).  

However, because {{DrillConnectionConfig}} is a class, the tracing proxy can't create a proxy
class wrapping it, and so can't trace calls to methods defined on {{DrillConnectionConfig}}
as it can for methods defined on other Drill JDBC driver objects.

There doesn't seem to be any valid reason for that limitation and inconsistency.  (At deeper
levels, we'd run into classes possibly with greater justification for staying/being classes,
but this {{DrillConnectionConfig}} is in {{org.apache.drill.jdbc}}, not deeper in the system.)

Additionally, there's the simple inconsistency that most of the types (classes or interfaces)
in the rest of the Drill JDBC interface, other than those that have to be classes (e.g., {{SQLException}}
subclasses and the driver class), are interfaces, but {{DrillConnectionConfig}} is not.

> I really don't see a good reason for changing all the classes to interfaces. 

Please note that I'm proposing changing only one class in this JIRA report.

> One of the effects similar refactoring has had is that history is getting lost. 

Perhaps there's a way to still record whatever part of history might actually be important
without sacrificing functionality (re proxying in this case), usability (re this case's being
in a published/external interface), and maintainability / easy testability?

What types/forms or cases of history loss are you thinking of?  




was (Author: dsbos):
> I would rather we did not change things that are not broken.

I'm trying to fix something that arguably _is_ broken:

{{TracingProxyDriver}} can trace calls across the rest of Drill's JDBC interface because it
can create proxy classes wrapping Drill's implementation classes, since they are defined in
terms of interfaces (e.g., JDBC's interface {{java.sql.Connection}} and Drill's derived interface
{{org.apache.drill.jdbc.DrillConnection}}).  

However, because {{DrillConnectionConfig}} is a class, the tracing proxy can't create a proxy
class wrapping it, and so can't trace calls to methods defined on {{DrillConnectionConfig}}
as it can for methods defined on other Drill JDBC driver objects.

There doesn't seem to be any valid reason for that limitation and inconsistency.  (At deeper
levels, we'd run into classes possibly with greater justification for staying/being classes,
but this {{DrillConnectionConfig}} is in {{org.apache.drill.jdbc}}, not deeper in the system.)

Additionally, there's the simple inconsistency that most of the types (classes or interfaces)
in the rest of the Drill JDBC interface, other than those that have to be classes (e.g., {{SQLException}}
subclasses and the driver class), are interfaces, but {{DrillConnectionConfig}} is not.

> I really don't see a good reason for changing all the classes to interfaces. 

Please note that I'm proposing changing only one class in this JIRA report.

> One of the effects similar refactoring has had is that history is getting lost. 

Perhaps there's a way to still record whatever part of history might actually be important
without sacrificing functionality (re proxying in this case), usability (re this case's being
in a published/external interface), and maintainability / easy testability?

What types/forms or cases of history loss are you thinking of?  



> Change JDBC driver's DrillConnectionConfig to interface
> -------------------------------------------------------
>
>                 Key: DRILL-3730
>                 URL: https://issues.apache.org/jira/browse/DRILL-3730
>             Project: Apache Drill
>          Issue Type: Bug
>          Components: Client - JDBC
>            Reporter: Daniel Barclay (Drill)
>             Fix For: Future
>
>
> Change {{org.apache.drill.jdbc.DrillConnectionConfig}} (in Drill's published interface
for the JDBC driver) from being a class to being an interface.
> Move the implementation (including the inheritance from {{net.hydromatic.avatica.ConnectionConfigImpl}})
from published-interface package {{org.apache.drill.jdbc}} to a class in implementation package
> {{org.apache.drill.jdbc.impl}}.
> (See "TODO(DRILL-3730)" mark currently in DrillConnectionConfig.)



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message