You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/02/08 14:48:50 UTC

[GitHub] [ignite] timoninmaxim commented on pull request #8490: IGNITE-13056 Move indexes to core

timoninmaxim commented on pull request #8490:
URL: https://github.com/apache/ignite/pull/8490#issuecomment-775201344


   Hi @korlov42 ! Thanks a lot for reviewing my PR! You ask right questions. I worked on this PR pretty much time and then need to restore why I did some things this way. So I'm going to answer for one thing at a time.
   
   > dropping supporting of fixed-size and case insensitive strings
   
   Those 2 types actually aren't used in Ignite. 
   1. There is a ticket for supporting case insensitive things [IGNITE-3999](https://issues.apache.org/jira/browse/IGNITE-3999). Also some comments in this ticket suggest to implement it in different way, with introducing functional indexes.
   2. For fixed size strings. We work with them incorrectly and it looks like a bug. H2 provide a mapping: ValueStringFixed is used for char(), nchar(), character() types. But Ignite maps char() to String.class, then with H2Utils maps String.class to varchar, see [H2Utils.dbTypeFromClass](https://github.com/apache/ignite/blob/86073947248f0ca878e754e8b1b6181fdac72bd0/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/H2Utils.java#L709)
   
   I didn't have a problem to implement them, there will be just 2 classes that extends StringInlineIndexKeyType. But we can't test them now, as they will be not in use.
   
   > closes doors for future improvement like [this one](https://issues.apache.org/jira/browse/IGNITE-13364).
   
   For task IGNITE-13364, I check a PR (PR/8161)[https://github.com/apache/ignite/pull/8161/files] for this task and found that there is no problems to implement in with current changes. Actually there is `precision` field (GridH2RowDescriptor -> GridQueryProperty), that can be declared within IndexKeyDefinition, and used for computing inline size instead of parsing sql query.
   
   So, why do you think it will close door for new improvements?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org