Bring NRediSearch "even" with JRediSearch #1267
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
I checked the RediSearch source and found that it's been "Unsupported language" since at least August 28, 2018.
Index optimizations are now handled by the internal garbage collector in the background. https://oss.redislabs.com/redisearch/Commands.html#ftoptimize
Made the private `args` member naming more consistent the rest of the project by prefixing with an underscore.
Java version had.
`getArgsString`
Kept everything as close to the original as possible.
AggregationBuilder.
Marked the obsolete unit test as obsolete.
setup portion of this test.
Fixed some assertions...
|
I must have overlooked the This will be fun... [Update] I was just looking at the JRediSearch unit test wrong... Fixed. |
anything to inherit from it. Cleaned up how we're ensuring that an object to compare is not null and is of an appropriate type. Fixed equality check so that it doesn't blow up on null values (payload specifically).
to GetInfoParsed and GetInfoParsedAsync.
Added in `SetNoStopwords` method to match the JRediSearch api as well as provide a conveinient means for keeping the default index options AND specifying that no stopwords be considered. Fixed `TestStopwords` unit test by specifying `STOPWORDS 0` by calling `SetNoStopwords` which adds the `DisableStopWords` option to the configured index options.
your index definitions.
the new version which leverages the suggestion builder. Added a small covering test.
|
It looks like the AggregationBuilder added the ability to FILTER and CURSOR. It shouldn't be an issue to port that to the AggregationRequest if that's the direction you wanna take. As for why the AggregationRequest was replaced by the builder... Perhaps some kind of idiomatic Java practice? |
|
@tombatron it was replaced as part of a code "refactoring" |
|
p.s. @tombatron thanks for the great work, it looks great |
|
Appreciate the input here - the code and the eyeballs. I'm AFK for a bit
over the holiday season, but let's try and get this merged ASAP in January.
…On Fri, 20 Dec 2019, 06:03 Guy Korland, ***@***.***> wrote:
p.s. @tombatron <https://github.com/tombatron> thanks for the great work,
it looks great
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1267?email_source=notifications&email_token=AAAEHMDGQZNTUYQDOFV7S3DQZRN4LA5CNFSM4JI3DDOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHL7MKY#issuecomment-567801387>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAEHMBEZGJIZUCRXW75ZXDQZRN4LANCNFSM4JI3DDOA>
.
|
|
Have a good holiday my man!
On Sun, Dec 22, 2019 at 9:52 AM Marc Gravell <notifications@github.com>
wrote:
… Appreciate the input here - the code and the eyeballs. I'm AFK for a bit
over the holiday season, but let's try and get this merged ASAP in January.
On Fri, 20 Dec 2019, 06:03 Guy Korland, ***@***.***> wrote:
> p.s. @tombatron <https://github.com/tombatron> thanks for the great
work,
> it looks great
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <
#1267?email_source=notifications&email_token=AAAEHMDGQZNTUYQDOFV7S3DQZRN4LA5CNFSM4JI3DDOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHL7MKY#issuecomment-567801387
>,
> or unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/AAAEHMBEZGJIZUCRXW75ZXDQZRN4LANCNFSM4JI3DDOA
>
> .
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1267?email_source=notifications&email_token=AAA5ESIDMX3ISYFSY6N7EYTQZ55L3A5CNFSM4JI3DDOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHPRZCY#issuecomment-568269963>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA5ESPENLA2MHZ3WL3JS7LQZ55L3ANCNFSM4JI3DDOA>
.
|
|
Note: I've added NRediSearch to the Linux test suite in #1316 - once that's merged, we should pull it in here to get PR testing :) |
|
Awesome! Happy New Year!
…On Wed, Jan 1, 2020 at 11:00 AM Nick Craver ***@***.***> wrote:
Note: I've added NRediSearch to the Linux test suite in #1316
<#1316> - once
that's merged, we should pull it in here to get PR testing :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1267?email_source=notifications&email_token=AAA5ESORQETRUAMA2L323WLQ3S42HA5CNFSM4JI3DDOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH5HW6I#issuecomment-570063737>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA5ESKW7PKCA6WAXI4YFQLQ3S42HANCNFSM4JI3DDOA>
.
|
|
Merging master in so we light up testing! |
|
Sweet!
…On Mon, Jan 6, 2020 at 6:34 PM Nick Craver ***@***.***> wrote:
Merging master in so we light up testing!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1267?email_source=notifications&email_token=AAA5ESKQMUZTV4KS6XL64JTQ4O5ZVA5CNFSM4JI3DDOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIHEZHY#issuecomment-571362463>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA5ESP2F2MHLB6DW4A4CBTQ4O5ZVANCNFSM4JI3DDOA>
.
|
|
Hey @NickCraver is there anything outstanding that I need to do, or are we just waiting on some extra bandwidth? |
|
Looking great, thanks; merging |
|
Thanks @mgravell ! It was a little intimidating opening the PR but you and @NickCraver really made it as painless as possible. |
|
no, thank you; there's a lot of work here that I didn't have to do, which is great. |
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.
Ported AggregationBuilder with Tests from JRediSearch.
Added Cursor support to the Client and AggregationResult.
Added RETURN support to the Query class.
Added INKEYS support to the Query class.
Added support for the FT.ALTER command.
Introduced new result type for the FT.INFO command. Removed TODO.
Updated “TestHighlightSummarize” test to match that of JRediSearch.
Added support for the FT.MGET command.
Expanded support for suggestions. Ported Suggestion class (with builder) and SuggestionOptions class (with builder).
Added ability to add multiple documents to an index with a single call.
Added ability to specify that an underlying indexed document be deleted when calling
DeleteDocument.Added ability to specify multiple documents for deletion.
Refactored IndexOptions to allow it to behave more like JRediSearch. Removed NOSCOREIDX index option as that has been deprecated.