[ 
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)

Reply via email to