-
Notifications
You must be signed in to change notification settings - Fork 509
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-6750 Fix numeric range syntax #4505
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4505 +/- ##
==========================================
+ Coverage 85.80% 85.94% +0.14%
==========================================
Files 188 190 +2
Lines 33107 34435 +1328
==========================================
+ Hits 28406 29595 +1189
- Misses 4701 4840 +139 ☔ 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.
Looks good 👍🏼 Few comments
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.
Few more comments
src/numeric_filter.c
Outdated
int sign, QueryError *status) { | ||
if (isMin && ( | ||
(sign == 1 && !strcasecmp(s, "-inf")) || | ||
(sign == -1 && (!strcasecmp(s, "+inf") || !strcasecmp(s, "inf"))))) { |
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.
Consider avoiding 2 library calls
(sign == -1 && (!strcasecmp(s, "+inf") || !strcasecmp(s, "inf"))))) { | |
(sign == -1 && !strcasecmp((*s == '+' ? s + 1 : s), "inf")))) { |
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.
Done. Thanks.
src/numeric_filter.c
Outdated
*target = NF_NEGATIVE_INFINITY; | ||
return REDISMODULE_OK; | ||
} else if (!isMin && !strcasecmp(s, "+inf")) { | ||
} else if (!isMin && ( | ||
(sign == 1 && (!strcasecmp(s, "+inf") || !strcasecmp(s, "inf"))) || |
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.
Consider avoiding 2 library calls
(sign == 1 && (!strcasecmp(s, "+inf") || !strcasecmp(s, "inf"))) || | |
(sign == 1 && !strcasecmp((*s == '+' ? s + 1 : s), "inf")) || |
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.
Done. Thanks.
src/numeric_filter.c
Outdated
if(sign == -1) { | ||
*target = -(*target); | ||
} |
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.
Should move to after the following error check
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.
OK.
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.
👍🏼 😎
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!
See few comments
…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.
Looks good.
See 2 small comments.
tests/pytests/test_search_params.py
Outdated
env = Env(moduleArgs = 'DEFAULT_DIALECT 2') | ||
env = Env(moduleArgs = 'DEFAULT_DIALECT 5') |
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.
Consider adding a for loop over dialects 2 and 5 (as you did before) for the part that is common, and then adding another section with the part relevant only for dialect 5. This may help with clarifying the difference between them later on.
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.
OK. Done. Thanks.
tests/pytests/test_search_params.py
Outdated
env.expect('FT.SEARCH', 'idx', '@n:[+-$n -+$n]', 'PARAMS', 2, 'n', 1).error() | ||
env.expect('FT.SEARCH', 'idx', '@n:[++$n --$m]', 'PARAMS', 2, 'n', 1).error() |
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.
Is there any special error that is raised here (I suspect no)?
You can add the error().contains('...
)` function to verify the error thrown.
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.
No. there is not an special error message, only Syntax error
#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)
Fix numeric range syntax (cherry picked from commit c05367b)
* 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 <76992134+oshadmi@users.noreply.github.com> * 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 <76992134+oshadmi@users.noreply.github.com>
* 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 <76992134+oshadmi@users.noreply.github.com> * 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 <76992134+oshadmi@users.noreply.github.com> (cherry picked from commit e907ece)
Description
This PR includes the following changes, which were included in parser v3
DIALECT 5
:[-$param1 +$param1]
[1 20e+5]
[-INF Inf]
Additional changes:
inf
(without sign) and not only+inf
or-inf
.filter score -INF Inf
This PR requires:
#4604 which creates the parser-v3
Not included in this PR:
[10 (((20]
(should only have a single leading open parenthesis for “exclusive” number)
will be done in MOD-6623: Querying NUMERIC with exact matching #4486
[(10 (10] - Breaking - keep current behavior - it means a range with no values - can
[(10]- Breaking
will be done in MOD-6623: Querying NUMERIC with exact matching #4486
Which issues this PR fixes
Main objects this PR modified
Mark if applicable