-
Notifications
You must be signed in to change notification settings - Fork 508
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
MOD-6749: Querying numeric fields using simple operators #4516
Conversation
This reverts commit 6c3ec77.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## nafraf_parser-v3-punct #4516 +/- ##
==========================================================
+ Coverage 85.94% 85.97% +0.02%
==========================================================
Files 190 190
Lines 34435 34517 +82
==========================================================
+ Hits 29596 29676 +80
- Misses 4839 4841 +2 ☔ View full report in Codecov by Sentry. |
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.
Cool! A few comments
src/query_parser/v2/parser.y
Outdated
|
||
expr(A) ::= modifier(B) COLON NOT_EQUAL param_num(C). { | ||
if (C.type == QT_PARAM_NUMERIC) { | ||
C.type = QT_PARAM_NUMERIC_MIN_RANGE; |
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.
For !=
could also be QT_PARAM_NUMERIC_MAX_RANGE
?
Does it mean a parameter can be substituted with only one of -inf
or +inf
?
Difference between any value and -inf
or +inf
is always evaluated to 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.
It was an error, we don't need to check the min value. Tests were added with parameters equals to +/-inf
. See e0e0b4d
src/query_parser/v2/parser.y
Outdated
|
||
numeric_operator(A) ::= EQUAL EQUAL param_num(B). { | ||
if (B.type == QT_PARAM_NUMERIC) { | ||
B.type = QT_PARAM_NUMERIC_MIN_RANGE; |
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.
Same question for ==
could also be QT_PARAM_NUMERIC_MAX_RANGE
?
Does it mean a parameter can be substituted with only one of -inf
or +inf
?
Equality of any value with -inf
or +inf
is never evaluated to 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.
That validation was removed and test with parameters equals to +/-inf
were added. See e0e0b4d
src/query_parser/v2/parser.y
Outdated
|
||
numeric_operator(A) ::= GREATER param_num(B). { | ||
if (B.type == QT_PARAM_NUMERIC) { | ||
B.type = QT_PARAM_NUMERIC_MIN_RANGE; |
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.
Does it mean a parameter can be substituted with only one of -inf
or +inf
?
Same question for the rest of the operators.
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 validation was removed.
tests/pytests/test_numbers.py
Outdated
res1 = env.cmd('FT.SEARCH', 'idx', '@n:==-15', 'NOCONTENT') | ||
env.assertEqual(res1, [0]) | ||
|
||
res1 = env.cmd('FT.SEARCH', 'idx', '@n:==-15', 'NOCONTENT') | ||
env.assertEqual(res1, [0]) |
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.
Duplicated?
res1 = env.cmd('FT.SEARCH', 'idx', '@n:==-15', 'NOCONTENT') | |
env.assertEqual(res1, [0]) | |
res1 = env.cmd('FT.SEARCH', 'idx', '@n:==-15', 'NOCONTENT') | |
env.assertEqual(res1, [0]) | |
res1 = env.cmd('FT.SEARCH', 'idx', '@n:==-15', 'NOCONTENT') | |
env.assertEqual(res1, [0]) |
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.
Yes. Removed.
tests/pytests/test.py
Outdated
env.assertEqual(res1, expected) | ||
|
||
if not env.isCluster(): | ||
# FT.EXPLAINCLI is not supported by the coordinator |
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.
Can be united with testing FT.EXPLAIN
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 united in function _testExplain()
…raf_fix-num-range-syntax
…raf_fix-num-range-syntax
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.
Muy Rico! 🍗
numeric_operator(A) ::= EQUAL_EQUAL param_num(B). { | ||
A = NewNumericFilterQueryParam_WithParams(ctx, &B, &B, 1, 1); | ||
} | ||
|
||
numeric_operator(A) ::= GT param_num(B). { | ||
A = NewNumericFilterQueryParam_WithParams(ctx, &B, NULL, 0, 1); | ||
} | ||
|
||
numeric_operator(A) ::= GE param_num(B). { | ||
A = NewNumericFilterQueryParam_WithParams(ctx, &B, NULL, 1, 1); | ||
} | ||
|
||
numeric_operator(A) ::= LT param_num(B). { | ||
A = NewNumericFilterQueryParam_WithParams(ctx, NULL, &B, 1, 0); | ||
} | ||
|
||
numeric_operator(A) ::= LE param_num(B). { | ||
A = NewNumericFilterQueryParam_WithParams(ctx, NULL, &B, 1, 1); | ||
} |
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.
Nice 🔥
#4604) * Create parser v3 * Test dialect: DEFAULT_DIALECT as module arg * More tests for text queries * Improve invalid syntax text * Fix parser v3, unary op after field name * Add missint test with modifierlist * WIP: Test isempty() with DIALECT 5 * test_v1_vs_v2_vs_v5() * Add tests to improve codecov using dialect 5 * One more test to improve codecov * Add WITHCOUNT to fix test with DIALECT 5 * Fix testEmptyValueTags() for DIALECT > 2 * Fix test_tags * Test float without leading zero * Fix wrong float number test * Test ragel minization at the end of compilation * Revert "Test ragel minization at the end of compilation" This reverts commit 0dae78b. * Fix make-parser.mk * cpp-test parser v3 * Fix float number syntax, leading zero is optional * Test number format * Support numbers with multiple signs * Create macros in parser.y v3 * Use set_max_dialect * Fix testEmptyValueTags() * MOD-6750 Fix numeric range syntax (#4505) Fix numeric range syntax * MOD-6749: Querying numeric fields using simple operators (#4516)
(cherry picked from commit 4ea8fc8)
* Add query_parser/v3 * Add more tag tests * Add test for TEXT testExact * autoescaping single tag between brackets * Fix tests COORD=1 * Fix wildcard support * wildcard + prefix/infix/suffix is invalid * Test backward compatibility * Test some uncovered cases * Test prefix/infix/suffix with TEXT field * Remove temporary debug messages * Test: escape 'w' single_tag * Add more tag tests * WIP: Tests to increase coverage parser v3 * Add more tests * Fix lexer.c v3 * Fix test invalid syntax * Test punct and cntrl characters * split unescaped_tag rule * Add one more test UNESCAPED_TAG * Fix expected test format in cluster * Update src/query_parser/v3/lexer.rl - Fix description Co-authored-by: Omer Shadmi <[email protected]> * Test pipe with dialect < 5, add comment about backslack escaping * Test escaping $ * One more test escaping $ * lexer v3 - remove leading and trailing spaces * Test short tags * Add JSON tests * Use comma separator for JSON tests * Add test testTagUNF() * Test tag autoescaping using DEFAULT_DIALECT 5 * More test using DEFAULT DIALECT 5 in test_search_params: test_geo, test_attr, test_binary_data. * Revert changes to test_search_params:test_geo * testTagUNF: Create index before hashes * Revert change in QueryNode_DumpSds() * Test aggregate with TAG autoescaping * Fix testTagAutoescaping, remove additional right curly brace * Update testDialect5InvalidSyntax() * update parser/v3 taking latest parser/v2 * Create parser v3 * Test dialect: DEFAULT_DIALECT as module arg * More tests for text queries * Improve invalid syntax text * Fix parser v3, unary op after field name * Add missint test with modifierlist * WIP: Test isempty() with DIALECT 5 * test_v1_vs_v2_vs_v5() * Add tests to improve codecov using dialect 5 * One more test to improve codecov * Add WITHCOUNT to fix test with DIALECT 5 * Fix testEmptyValueTags() for DIALECT > 2 * Fix test_tags * Test float without leading zero * Fix wrong float number test * Test ragel minization at the end of compilation * Revert "Test ragel minization at the end of compilation" This reverts commit 0dae78b. * Fix make-parser.mk * cpp-test parser v3 * Update parser v3 * Update lexer * Fix tests * Fix float number syntax, leading zero is optional * Test number format * Support numbers with multiple signs * Create macros in parser.y v3 * Use set_max_dialect * Fix testEmptyValueTags() * MOD-6750 Fix numeric range syntax (#4505) Fix numeric range syntax * MOD-6749: Querying numeric fields using simple operators (#4516) * Simplify single_tag * Remove unescaped_tag2 * lexer v3 - remove colon from tag expressions * Create unescaped_tag2 to create UNESCAPED_TAG without escape * Fix leading/trailing spaces deletion * Validate tok.len * Remove debugging code * Fix lexer.rl v3 format * Temp: Try to fix sanitizer * Revert "Temp: Try to fix sanitizer" This reverts commit 66002f1. * Test tag with * as literal * Fix parser: tag rules * Update tests/pytests/test_tags.py - minor typo --------- Co-authored-by: Omer Shadmi <[email protected]>
* Add query_parser/v3 * Add more tag tests * Add test for TEXT testExact * autoescaping single tag between brackets * Fix tests COORD=1 * Fix wildcard support * wildcard + prefix/infix/suffix is invalid * Test backward compatibility * Test some uncovered cases * Test prefix/infix/suffix with TEXT field * Remove temporary debug messages * Test: escape 'w' single_tag * Add more tag tests * WIP: Tests to increase coverage parser v3 * Add more tests * Fix lexer.c v3 * Fix test invalid syntax * Test punct and cntrl characters * split unescaped_tag rule * Add one more test UNESCAPED_TAG * Fix expected test format in cluster * Update src/query_parser/v3/lexer.rl - Fix description Co-authored-by: Omer Shadmi <[email protected]> * Test pipe with dialect < 5, add comment about backslack escaping * Test escaping $ * One more test escaping $ * lexer v3 - remove leading and trailing spaces * Test short tags * Add JSON tests * Use comma separator for JSON tests * Add test testTagUNF() * Test tag autoescaping using DEFAULT_DIALECT 5 * More test using DEFAULT DIALECT 5 in test_search_params: test_geo, test_attr, test_binary_data. * Revert changes to test_search_params:test_geo * testTagUNF: Create index before hashes * Revert change in QueryNode_DumpSds() * Test aggregate with TAG autoescaping * Fix testTagAutoescaping, remove additional right curly brace * Update testDialect5InvalidSyntax() * update parser/v3 taking latest parser/v2 * Create parser v3 * Test dialect: DEFAULT_DIALECT as module arg * More tests for text queries * Improve invalid syntax text * Fix parser v3, unary op after field name * Add missint test with modifierlist * WIP: Test isempty() with DIALECT 5 * test_v1_vs_v2_vs_v5() * Add tests to improve codecov using dialect 5 * One more test to improve codecov * Add WITHCOUNT to fix test with DIALECT 5 * Fix testEmptyValueTags() for DIALECT > 2 * Fix test_tags * Test float without leading zero * Fix wrong float number test * Test ragel minization at the end of compilation * Revert "Test ragel minization at the end of compilation" This reverts commit 0dae78b. * Fix make-parser.mk * cpp-test parser v3 * Update parser v3 * Update lexer * Fix tests * Fix float number syntax, leading zero is optional * Test number format * Support numbers with multiple signs * Create macros in parser.y v3 * Use set_max_dialect * Fix testEmptyValueTags() * MOD-6750 Fix numeric range syntax (#4505) Fix numeric range syntax * MOD-6749: Querying numeric fields using simple operators (#4516) * Simplify single_tag * Remove unescaped_tag2 * lexer v3 - remove colon from tag expressions * Create unescaped_tag2 to create UNESCAPED_TAG without escape * Fix leading/trailing spaces deletion * Validate tok.len * Remove debugging code * Fix lexer.rl v3 format * Temp: Try to fix sanitizer * Revert "Temp: Try to fix sanitizer" This reverts commit 66002f1. * Test tag with * as literal * Fix parser: tag rules * Update tests/pytests/test_tags.py - minor typo --------- Co-authored-by: Omer Shadmi <[email protected]> (cherry picked from commit e907ece)
Description
Add support to new syntax for numeric field query, simple operators:
==
,!=
,>
,<
,>=
,<=
The operators are only valid using DIALECT 5.
This PR requires:
#4604 which creates the parser-v3
#4505 fix numeric range syntax
Numeric operators does not require
:
, but it is still required to specify a range:Example:
Which issues this PR fixes
Main objects this PR modified
Mark if applicable