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
fixed case issue MOD-4872 #3403
Conversation
Codecov ReportBase: 82.72% // Head: 82.65% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3403 +/- ##
==========================================
- Coverage 82.72% 82.65% -0.07%
==========================================
Files 175 175
Lines 29921 29940 +19
==========================================
- Hits 24752 24748 -4
- Misses 5169 5192 +23
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@@ -213,7 +213,7 @@ QueryNode *NewPrefixNode_WithParams(QueryParseCtx *q, QueryToken *qt, bool prefi | |||
ret->pfx.suffix = suffix; | |||
q->numTokens++; | |||
if (qt->type == QT_TERM) { | |||
char *s = rm_strdupcase(qt->s, qt->len); | |||
char *s = rm_strndup_unescape(qt->s, qt->len); |
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.
Need to also handle PARAMS
, such as this test
(QT_TERM_CASE
and QT_PARAM_TERM_CASE
?)
n->children = array_ensure_append(n->children, children + ii, 1, QueryNode *); | ||
for(size_t jj = 0; jj < QueryNode_NumParams(child); ++jj) { | ||
Param *p = child->params + jj; | ||
p->type = PARAM_TERM_CASE; |
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.
When evaluating PARAM_TERM_CASE
we use rm_strdup
and not rm_strndup_unescape
Line 176 in 7eae2f6
*(char**)param->target = rm_strdup(val); |
So we do not unescape params?
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 handle un-escaping params in another PR
* fixed case issue * rename function + fix test * param bigfix + test * Add waitForIndex in test:testGroupByWithApplyError * Fix for test_short_read failure * test_short_read fix 2 --------- Co-authored-by: Omer Shadmi <76992134+oshadmi@users.noreply.github.com> Co-authored-by: rafie <rafi@redislabs.com> (cherry picked from commit a21efd6)
This PR removes redunant "to lower" transformation for prefix nodes and introduces
rm_strndupescape
to duplicate and escape strings.The prefix string will be modify to lower case when in
query_EvalSingleTagNode
calls totag_strtolower
when tag field is case sensitiveQuery_EvalPrefixNode
callsstrToFoldedRunes
which transform the query string into lower case regardlessadded test
closes MOD-4872, closes #3391