MOD-2972: Initial support for multi-value text #2819
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ashtul
reviewed
Jul 4, 2022
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
| ''' | ||
|
|
||
|
|
||
| def expect_undef_order(query : Query): |
There was a problem hiding this comment.
Unbound name: Name Query is used but not defined in the current scope.
Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
Codecov Report
@@ Coverage Diff @@
## master #2819 +/- ##
==========================================
+ Coverage 81.92% 81.96% +0.03%
==========================================
Files 180 180
Lines 29264 29425 +161
==========================================
+ Hits 23974 24117 +143
- Misses 5290 5308 +18
Continue to review full report at Codecov.
|
rafie
approved these changes
Jul 27, 2022
| @@ -727,6 +726,7 @@ def testconfigMultiTextOffsetDeltaSlopNeg(env): | |||
| err_msg = 'module init should fail due to invalid module configuration' | |||
|
|
|||
| env.assertIsNotNone(err_msg) | |||
| env = Env() | |||
There was a problem hiding this comment.
Why do you creating a new env here?
There was a problem hiding this comment.
It was getting a
ConnectionRefusedError: [Errno 111] Connection refused
And then
redis.exceptions.RedisClusterException: Redis Cluster cannot be connected. Please provide at least one reachable node.
got error on cluster info, will try again, Error 99 connecting to localhost:6379. Cannot assign requested address.
There was a problem hiding this comment.
So you are hiding the test failure?
oshadmi
added a commit
that referenced
this pull request
Aug 8, 2022
* WIP initial support for multi-value text * Add test for non-text values * WIP sorting * WIP Add JSON API V2 and avoid SORTABLE with multi value * add tests and avoid failing SORTABLE * check undefined ordering * Add FT config option MULTI_TEXT_OFFSET_DELTA * cleanups * code review with Ariel (1) * More fixes and code review with Ariel * Fix from review: rename config * [skip ci] Fix from review: refactor querying jsonpath info (1) * Fix from review: refactor querying jsonpath info (2) * Rename test file and fix log message typo * Add test for Boolean TAG * Fail index creation on an invalid jsonpath * Fix order in testMultiTagBool * Remove duplicated entries from JSON API V2 * Make JSON API versions incremental * Make JSON API versions incremental (2) * API cleanup * Rename API pathIsStatic to pathIsSingle * update readies * Add alternative expected depending on ReJSON version * Fixes from Meir's review (1) * update readies and increase no_output_timeout * Add score to see ordering in failed test * Fix test for coordinator * Fix test for coordinator * Fix test for coordinator (2) * Fix tests (order and moduleArgs config param) * Iterate a json array without parsing a jsonpath * Skip score test in cluster * Fixes from Meir's review (2) - single Env per test, run on coord * Fixes from Meir's review (2) - revert timeout * Try skip MULTI_TEXT_SLOP config tests on cluster * Fix MULTI_TEXT_SLOP config tests on cluster * Fix MULTI_TEXT_SLOP config tests on cluster (2) * update readies
oshadmi
added a commit
that referenced
this pull request
Sep 21, 2022
* WIP initial support for multi-value text * Add test for non-text values * WIP sorting * WIP Add JSON API V2 and avoid SORTABLE with multi value * add tests and avoid failing SORTABLE * check undefined ordering * Add FT config option MULTI_TEXT_OFFSET_DELTA * cleanups * code review with Ariel (1) * More fixes and code review with Ariel * Fix from review: rename config * [skip ci] Fix from review: refactor querying jsonpath info (1) * Fix from review: refactor querying jsonpath info (2) * Rename test file and fix log message typo * Add test for Boolean TAG * Fail index creation on an invalid jsonpath * Fix order in testMultiTagBool * Remove duplicated entries from JSON API V2 * Make JSON API versions incremental * Make JSON API versions incremental (2) * API cleanup * Rename API pathIsStatic to pathIsSingle * update readies * Add alternative expected depending on ReJSON version * Fixes from Meir's review (1) * update readies and increase no_output_timeout * Add score to see ordering in failed test * Fix test for coordinator * Fix test for coordinator * Fix test for coordinator (2) * Fix tests (order and moduleArgs config param) * Iterate a json array without parsing a jsonpath * Skip score test in cluster * Fixes from Meir's review (2) - single Env per test, run on coord * Fixes from Meir's review (2) - revert timeout * Try skip MULTI_TEXT_SLOP config tests on cluster * Fix MULTI_TEXT_SLOP config tests on cluster * Fix MULTI_TEXT_SLOP config tests on cluster (2) * update readies (cherry picked from commit 602ae4b)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Indexing and Searching TEXT with single or multi value
Marked with label
pr:breaksince invalid JSONPath will now fail index creation (when a RedisJSON with JSON API V2 is loaded)Limitations:
SLOPandINORDERcannot be used (since the order of the values is not well defined)nullvalues are skipped, and any other value type is causing an indexing failureSORTBYis only sorting by the first valueHIGHLIGHTsupportRETURNof a Schema attribute, whose JSONPath leads to multiple values, returns only the first value (as a JSON String)RETURNinstead of a Schema attribute, all values are returned (as a JSON String)More details:
MULTI_TEXT_SLOP(at module load-time)Followup PR's: