[
https://issues.apache.org/jira/browse/CALCITE-912?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14945511#comment-14945511
]
Julian Hyde commented on CALCITE-912:
-------------------------------------
First impressions on the first patch:
* It seems right to allow a Meta to be shared among several connections, and
therefore I agree with adding a ConnectionHandle to the various getXxx metadata
methods.
* Generating statement IDs using Random.nextInt() is wrong.
* Removing 'final' from AvaticaConnection.id and .handle seems wrong.
* Please don't remove the code in "getProcedures" etc because "RemoteMeta does
not implement it either". One day RemoteMeta will implement it.
* The error handling is useful - thanks!
* The parameter to openConnection should not be a Properties (which is mutable)
and I'm not sure whether the values should be strings. I think a Map<String,
Object>.
* Overall there are too many TODOs and hacks.
> Pass URL connection properties to avatica server
> ------------------------------------------------
>
> Key: CALCITE-912
> URL: https://issues.apache.org/jira/browse/CALCITE-912
> Project: Calcite
> Issue Type: Improvement
> Components: avatica
> Affects Versions: 1.4.0-incubating, 1.5.0-incubating
> Reporter: Bruno Dumon
> Assignee: Julian Hyde
> Attachments: 0001-Avatica-conn-createion-with-properties.patch,
> 0001-Avatica-fix-remote-columns-iterations.patch
>
>
> This ticket proposes the ability to pass connection-creation-time properties
> (the "info") from the remote avatica driver to the avatica server.
> Use cases:
> * Phoenix properties like CurrentSCN and TenantId (and similar use-cases for
> a custom driver at the company I work for)
> * user and password properties (enables authentication, cfr. CALCITE-519 and
> CALCITE-643)
> See also discussion on mailing list:
> http://mail-archives.apache.org/mod_mbox/incubator-calcite-dev/201510.mbox/%3CCAAF1JdiZbRuMfo2CFRUoxPhBBf%3DyQ5T%2Bd%2B1ig1FzKnVDj8RGOA%40mail.gmail.com%3E
> Things to consider:
> * The remote driver might have its own properties (like "url") which should
> not be passed to the server. Remote driver is responsible for removing those.
> * The server might want to disallow setting some properties remotely, e.g.
> to not allow to connect to a different database.
> Currently the avatica server creates a default connection (though it is
> marked for removal). When using authentication, this is particularly
> annoying, since you need to start up the server with an url containing user
> and password.
> I will attach a patch, based on calcite 1.4, which adds the ability to pass
> properties as part of a new OpenConnectionRequest rpc call (a
> CreationConnetionRequest was proposed for different reasons in CALCITE-663).
> For the authentication reason mentioned, the default connection is also
> dropped. There is one JdbcMeta instance on the server side, but it manages a
> map of connections. Since connections were already created per client-side
> connection, this is not different than before, except that the connections
> are now created explicitly. Implicit connection creation has been dropped, we
> can look into auto-recreation of the connection as part of recovery
> (CALCITE-903).
> The somewhat hacky things in the patch are:
> * addition of AvaticaConnection.setId(): this is because I let the server
> decide on the connection id. This seemed more logical to me at first, though
> it can be easily reversed.
> * remote.Driver which creates an extra service instance to be able to do the
> OpenConnection request, while all other requests happen from within
> RemoteMeta.
> The patch also passes error messages back to the client (a simple improvement
> towards CALCITE-645).
> Because the calls for metadata (such as getColumns) now also have a
> connection id, fixing CALCITE-871 becomes easy and I have a patch ready for
> that as well.
> If the patch is acceptable, I can rework it for master (now it is for 1.4).
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
