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:
us...@infra.apache.org


Reply via email to