Skip to content
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

Java: Adding command LCS (No IDX option) #1558

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

tjzhang-BQ
Copy link
Collaborator

@tjzhang-BQ tjzhang-BQ commented Jun 12, 2024

  • Java: Adding command LCS (default & LEN option)

Issue #, if available:

Description of changes:
This only covers the signature where LCS returns the LCS as a String and LCS length as a Long. Map return type with the IDX option is coming as a part II

MINMATCHLEN does not seem to work:

127.0.0.1:6379> set a1 abc
OK
127.0.0.1:6379> set a2 bcd
OK
127.0.0.1:6379> lcs a1 a2
"bc"
127.0.0.1:6379> lcs a1 a2 len
(integer) 2
127.0.0.1:6379> lcs a1 a2 MINMATCHLEN 3
"bc"
127.0.0.1:6379> lcs a1 a2 len MINMATCHLEN 3
(integer) 2
127.0.0.1:6379> lcs a1 a2 idx MINMATCHLEN 3
1) "matches"
2) (empty array)
3) "len"
4) (integer) 2

And as a reference valkey-io/valkey#420 discusses how WITHMATCHLEN and MINMATCHLEN only works/makes sense with the IDX option but is still allowed to be added when IDX isn't present.

For now, this PR will only include the default behavior without optional keywords and the LEN option behavior, which is mutually exclusive with IDX, without the MINMATCHLEN option

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@tjzhang-BQ tjzhang-BQ requested a review from a team as a code owner June 12, 2024 23:06
* Java: Adding command LCS (default & LEN option)
@Yury-Fridlyand Yury-Fridlyand added the java issues and fixes related to the java client label Jun 12, 2024
@shohamazon
Copy link
Collaborator

the best way rn is to add this option only when IDX is passed

@acarbonetto acarbonetto changed the title Java: Adding command LCS (No IDX option) (#351) Java: Adding command LCS (No IDX option) Jun 13, 2024
* subsequences.
* @example
* <pre>{@code
* // testKey1 = abcd, testKey2 = axcd
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it not easier to just include: client.mset({"testKey1", "testKey2"}, {"abcd", "axcd"}).get();

@acarbonetto acarbonetto merged commit ed84725 into valkey-io:main Jun 13, 2024
46 checks passed
@acarbonetto acarbonetto deleted the java/integ_tjz_lcs_I branch June 13, 2024 21:03
cyip10 pushed a commit to Bit-Quill/valkey-glide that referenced this pull request Jun 24, 2024
Java: Adding command `LCS` (No IDX option) (#351)

* Java: Adding command LCS (default & LEN option)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
java issues and fixes related to the java client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants