ignite-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vladimir Ozerov (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (IGNITE-6055) SQL: Add String length constraint
Date Tue, 14 Aug 2018 10:23:00 GMT

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

Vladimir Ozerov edited comment on IGNITE-6055 at 8/14/18 10:22 AM:
-------------------------------------------------------------------

[~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.


was (Author: vozerov):
[~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)

Mime
View raw message