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

Vladimir Ozerov commented on IGNITE-6055:
-----------------------------------------

[~NIzhikov], my comments:


 1) {{IgniteQueryErrorCode}} - tow new codes were added, but they are not 
converted to SqlState in {{codeToSqlState}} method. In addition, there should 
be new JDBC tests confirming that proper SQL state is returned.
 2) {{QueryUtils.buildBinaryProperty}} - unused parameter {{cacheName}}
 3) What is the purposes of changes in REST classes 
({{GridClientHandshakeResponse}}, {{GridTcpRestParser}})? I see that passed 
version is not used. Also note that REST client is not thin client, and thus 
should not use thin client versioning.
 4) Something is wrong with {{QueryUtils.processBinaryMeta}} - why validation 
of not-null fields is handled inside {{QueryBinaryProperty.addProperty}}, but 
validation of precision is handled in 
{{QueryBinaryProperty.addValidateProperty}}? This way you may end up in a 
situation when the same property is add to "validate" collection twice, if it 
is not-null and has prevision. Then if you use {{DROP COLUMN}} command, then 
only first property will be removed from "validate" collection, and *all* 
subsequent {{INSERT}} commands will fail. Please remove {{addValidateProperty}} 
method and make sure that validation properties are populatied in a single 
place. Also please add a test as follows: create not-null column with precision 
-> test both constraints -> drop column -> insert some row. Insert should pass.
 5) {{ClientRequestHandler}} should not write server version back, as it 
doesn't make sense for a client. Our protocol works as follows: client proposes 
communication version to the server. If server accepted this version, then 
{{true}} is returned. Otherwise server proposes another version, and client 
re-tries handshake with alternative version if possible. In any case, current 
server version should never be used in any decision on the client side. 
 6) Please find a way to avoid passing version to {{IBinaryRawWriteAware}}. 
This is general-purpose interface and we should not add irrelevant data to it. 
I would rather add internal transient field to {{CacheConfiguration}} or so. 
Alterantively, you may add extended version of {{IBinaryRawWriter}} (e.g. 
{{IBinaryRawWriterEx}}), which will expose required version.
 7) {{ClientSocket.cs}} - same as p.5. Server version should not be used. You 
should use version both server and client agreed upon.

 

Once these problems are addressed and tested we should ask other community 
memebers to fix ODBC, CPP thin client, Node.JS thin client, Python thin client.

> SQL: Add String length constraint
> ---------------------------------
>
>                 Key: IGNITE-6055
>                 URL: https://issues.apache.org/jira/browse/IGNITE-6055
>             Project: Ignite
>          Issue Type: Task
>          Components: sql
>    Affects Versions: 2.1
>            Reporter: Vladimir Ozerov
>            Assignee: Nikolay Izhikov
>            Priority: Major
>              Labels: sql-engine
>             Fix For: 2.7
>
>
> We should support {{CHAR(X)}} and {{VARCHAR{X}} syntax. Currently, we ignore 
> it. First, it affects semantics. E.g., one can insert a string with greater 
> length into a cache/table without any problems. Second, it limits efficiency 
> of our default configuration. E.g., index inline cannot be applied to 
> {{String}} data type as we cannot guess it's length.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to