[
https://issues.apache.org/jira/browse/IGNITE-6055?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16522719#comment-16522719
]
Nikolay Izhikov edited comment on IGNITE-6055 at 6/25/18 7:34 PM:
------------------------------------------------------------------
[~vozerov]
> 1)
> org.apache.ignite.internal.processors.query.h2.ddl.DdlStatementsProcessor#toQueryEntity
> - this check is redundant, we should check whether QueryEntity is valid on
> cache start, not inside DDL (to handle both DDL and cache create through API
> in the same place). See
> org.apache.ignite.internal.processors.query.QueryUtils#validateQueryEntity
Fixed.
> 2) I am not very happy with precision/scale/maxLength properties - too much
> of them. Instead, we can re-use "precision" for String length. H2 works this
> way (see their docs), so it should be ok for us as well. In fact, this is why
> GridSqlColumn.maxLength() return the same value as GridSqlColumn.precision()
Fixed.
> 3) QueryEntity - getters should return current value, without wrapping it
> into unmodifiable collection, because this is how users frequently use us -
> QueryEntity.getNotNullFields().add(...)
Fixed.
> 4) Let's split single decimal property into "scale" and "precision". Because
> single precision would be needed not only for strings, but for other data
> types as well (e.g. DOUBLE, REAL, BINARY)
Fixed.
> 6) QueryBinaryProperty, QueryTypeDescriptorImpl, QueryUtils - checks for
> "_KEY" and "_VAL" are illegal, because key/val field names could be overriden
> with QueryEntity.keyFieldName/valFieldName. These checks should be more
> generic
Fixed.
> 7) QueryBinaryProperty.value() - new logic around key/val should be removed
> as it breaks an invariant that only nested fields can be read/written here.
> Please find a way to perform a check on key/value without changing central
> field extraction logic.
FIxed.
> 8) We need more tests for cache API. E.g. I do not see tests for invoke().
Tests added.
> 5) I do not see how compatibility is handled in .NET - new fields are
> serialized unconditionally, meaning that we cannot talk to previous version.
> Am I wrong?
Do we have compatibility for a .Net client?
I don't see any conditional logic for a "default field value" implementation
[1].
Can you give me an example of such logic?
Please, review the PR.
[1]
[https://github.com/apache/ignite/commit/78e79e011f1bb017653e628587faf606cfd37510]
was (Author: nizhikov):
> 1)
> org.apache.ignite.internal.processors.query.h2.ddl.DdlStatementsProcessor#toQueryEntity
> - this check is redundant, we should check whether QueryEntity is valid on
> cache start, not inside DDL (to handle both DDL and cache create through API
> in the same place). See
> org.apache.ignite.internal.processors.query.QueryUtils#validateQueryEntity
Fixed.
> 2) I am not very happy with precision/scale/maxLength properties - too much
> of them. Instead, we can re-use "precision" for String length. H2 works this
> way (see their docs), so it should be ok for us as well. In fact, this is why
> GridSqlColumn.maxLength() return the same value as GridSqlColumn.precision()
Fixed.
> 3) QueryEntity - getters should return current value, without wrapping it
> into unmodifiable collection, because this is how users frequently use us -
> QueryEntity.getNotNullFields().add(...)
Fixed.
> 4) Let's split single decimal property into "scale" and "precision". Because
> single precision would be needed not only for strings, but for other data
> types as well (e.g. DOUBLE, REAL, BINARY)
Fixed.
> 6) QueryBinaryProperty, QueryTypeDescriptorImpl, QueryUtils - checks for
> "_KEY" and "_VAL" are illegal, because key/val field names could be overriden
> with QueryEntity.keyFieldName/valFieldName. These checks should be more
> generic
Fixed.
> 7) QueryBinaryProperty.value() - new logic around key/val should be removed
> as it breaks an invariant that only nested fields can be read/written here.
> Please find a way to perform a check on key/value without changing central
> field extraction logic.
FIxed.
> 8) We need more tests for cache API. E.g. I do not see tests for invoke().
Tests added.
> 5) I do not see how compatibility is handled in .NET - new fields are
> serialized unconditionally, meaning that we cannot talk to previous version.
> Am I wrong?
Do we have compatibility for a .Net client?
I don't see any conditional logic for a "default field value" implementation
[1].
Can you give me an example of such logic?
[1]
[https://github.com/apache/ignite/commit/78e79e011f1bb017653e628587faf606cfd37510]
> 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
>
> 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)
