[
https://issues.apache.org/jira/browse/IGNITE-5163?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16012139#comment-16012139
]
Vladimir Ozerov commented on IGNITE-5163:
-----------------------------------------
[~tledkov-gridgain], my comments:
1) {{BinaryWriterExImpl}} = why did we added dummy methods which do nothing
more, but delefate to another one? I would remove all added {{do}} mehotds, and
make relevant {{write}} methods public instead of package-private
2) {{GridBinaryMarshaller.JDK_MARSH}} - this constant should not be placed in
{{GridBinaryMarshaller}} class, as binary protocol doesn't interact with JDK
marshaller anyhow. Essentially, you build something on top of binary protocol,
so this constant (flag?) should be somewhere inside {{SqlListener}} stuff.
3) {{GridKernalContext}} and {{GridKernalContextImpl}} - make sure to properly
rename inner fields and javadocs as well
4) {{JdbcConnection}} - let's remove {{PROP_CACHE}} and {{cacheName}}
completely, as it goes agains our plans to make schema "shareable" between
caches. Instead, we should trakc current schema (through method setSchema())
and add it to SQL execute request. For now we will use schema name to derive
cache name on the server side, but when IGNITE-5054 is completed, even this
will be no longer necessary; make sure to change ODBC part in the same way
5) {{JdbcConnection.createStatement0}}, {{url}}, {{cacheName}} - unused methodы
6) {{JdbcConnection.timeout}} - looks like we never use it; let's print a
warning for now that setting timeout is of no effect, and create separate
ticket to handle it properly
7) {{JdbcConnection.getMetaData}} - throw unsupported exception instead of
returning {{null}}? let's create another linked ticket for this
8) {{JdbcConnection.transactionIsolation}} - print a warning about unsupported
feature, but do not throw an exception, because some external frameworks might
call this method unconditionally;
9) {{JdbcConnection.setReadOnly}} - "updates are not supported" - really?
anyway, according to spec this is just a hint for database, nothing more, so no
exceptions should be raised here
10) {{JdbcConnection.setCatalog}} - according to spec "If the driver does not
support catalogs, it will silently ignore this request."
11) {{JdbcConnection.prepareStatement}} - am I right that we have separate
tickets for this?
12) {{JdbcConnection.isValid}} - incorrect implementation, you should return
{{false}} if connection is closed, so {{ensureNotClosed}} is illegal here.
Let's create separate ticket where we will send ping requests to the server
when this method is called
13) {{JdbcConnection.getClientInfo}} - let's create separate ticket to support
it properly
14) {{JdbcTcpIo.handshake}} - looks like you do not close the socket if
handshake fails
15) New readers/writers - I completely lost on what is going on there; we
should not mix binary and sql stuff anyhow. SQL must be built on top, it means
composition, not inheritance with strange overrides.
> JDBC Driver: implement handshake for thin jdbc driver based on common
> odbc/jdbc protocol
> ----------------------------------------------------------------------------------------
>
> Key: IGNITE-5163
> URL: https://issues.apache.org/jira/browse/IGNITE-5163
> Project: Ignite
> Issue Type: Task
> Components: odbc, sql
> Affects Versions: 1.9
> Reporter: Taras Ledkov
> Assignee: Taras Ledkov
> Fix For: 2.1
>
>
> The GridClient must be removed from thin jdbc driver.
> It is the first step to unify JDBC & ODBC protocols.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)