-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IGNITE-16446: Fix IndexQuery incorrect work with indexes built for _VAL field #9788
IGNITE-16446: Fix IndexQuery incorrect work with indexes built for _VAL field #9788
Conversation
|
||
/** */ | ||
private boolean isKeyField(String fld) { | ||
return fld.equals(typeDesc.keyFieldAlias()) || fld.equals(QueryUtils.KEY_FIELD_NAME); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
equalsIgnoreCase
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fld
, keyFieldAlias
are already normalized here, so it is impossible to fail with getting non-normalized keys here.
But, also we should keep in mind the CacheConfiguration#sqlEscapeAll()
flag. In case it set up, a user have to specify field in the same way as it was declared. For example, if field declared as "f1", then with sqlEscapeAll=false
it will be normalized to "F1" and user can query it with both "f1" or "F1". But if sqlEscapeAll=true
, then field will be "f1" and user has to run query with "f1" attribute only, and it's prohibited to use "F1" for querying.
So String#equals
works in both cases with or without escaping. I added tests for that.
|
||
/** */ | ||
private boolean isValueField(String fld) { | ||
return fld.equals(typeDesc.valueFieldAlias()) || fld.equals(QueryUtils.VAL_FIELD_NAME); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
equalsIgnoreCase
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same.
.../indexing/src/test/java/org/apache/ignite/cache/query/IndexQueryCacheKeyValueFieldsTest.java
Outdated
Show resolved
Hide resolved
|
||
/** */ | ||
@Test | ||
public void testKeyField() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also add tests with upper/lower case letters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added tests for cases and escaping.
/** */ | ||
@Parameterized.Parameters(name = "escape={0}") | ||
public static Object[] params() { | ||
return new Object[] { false, true }; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test is not parametrized, looks like this is redundant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
IndexQuery handles indexes built for single _VAL (or alias) field correctly. Now it checks field type before sorting on an originating node.