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

Revert make KEYS to be an exact match if there is no pattern #964

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

enjoy-binbin
Copy link
Member

In #792, the time complexity became ambiguous, fluctuating between
O(1) and O(n), which is a significant difference. And we agree uncertainty
can potentially bring disaster to the business, the right thing to do is
to persuade users to use EXISTS instead of KEYS in this case, to do the
right thing the right way, rather than accommodating this incorrect usage.

This reverts commit d66a06e.
This reverts #792.

In valkey-io#792, the time complexity became ambiguous, fluctuating between
O(1) and O(n), which is a significant difference. And we agree uncertainty
can potentially bring disaster to the business, the right thing to do is
to persuade users to use EXISTS instead of KEYS in this case, to do the
right thing the right way, rather than accommodating this incorrect usage.

This reverts commit d66a06e.
This reverts valkey-io#792.

Signed-off-by: Binbin <binloveplay1314@qq.com>
@enjoy-binbin enjoy-binbin merged commit 75b8240 into valkey-io:unstable Aug 29, 2024
44 of 45 checks passed
@enjoy-binbin enjoy-binbin deleted the revert_keys branch August 29, 2024 02:58
@zuiderkwast
Copy link
Contributor

We need to backpport this to the 8.0 branch? Adding to 8.0 project.

@enjoy-binbin
Copy link
Member Author

We need to backpport this to the 8.0 branch? Adding to 8.0 project.

ok, now we are already backporting 8.0? I thought we would merge unstable directly into 8.0, since all current changes will be for 8.0.

@soloestoy
Copy link
Member

put it to Optional for next RC is OK?

@zuiderkwast
Copy link
Contributor

We need to backpport this to the 8.0 branch? Adding to 8.0 project.

ok, now we are already backporting 8.0? I thought we would merge unstable directly into 8.0, since all current changes will be for 8.0.

I don't know actually. I was thinking at the time we create the 8.0 branch and we continue to merge more stuff into unstable, it means it will not automatically be in 8.0.

put it to Optional for next RC is OK?

If it's optional, it means maybe we will release 8.0 without the revert? It's not good. We need a new column for "backported" or "to be backported".

madolson pushed a commit that referenced this pull request Sep 2, 2024
In #792, the time complexity became ambiguous, fluctuating between
O(1) and O(n), which is a significant difference. And we agree uncertainty
can potentially bring disaster to the business, the right thing to do is
to persuade users to use EXISTS instead of KEYS in this case, to do the
right thing the right way, rather than accommodating this incorrect usage.

This reverts commit d66a06e.
This reverts #792.

Signed-off-by: Binbin <binloveplay1314@qq.com>
madolson pushed a commit that referenced this pull request Sep 3, 2024
In #792, the time complexity became ambiguous, fluctuating between
O(1) and O(n), which is a significant difference. And we agree uncertainty
can potentially bring disaster to the business, the right thing to do is
to persuade users to use EXISTS instead of KEYS in this case, to do the
right thing the right way, rather than accommodating this incorrect usage.

This reverts commit d66a06e.
This reverts #792.

Signed-off-by: Binbin <binloveplay1314@qq.com>
PingXie pushed a commit to PingXie/valkey that referenced this pull request Sep 14, 2024
…io#964)

In valkey-io#792, the time complexity became ambiguous, fluctuating between
O(1) and O(n), which is a significant difference. And we agree uncertainty
can potentially bring disaster to the business, the right thing to do is
to persuade users to use EXISTS instead of KEYS in this case, to do the
right thing the right way, rather than accommodating this incorrect usage.

This reverts commit d66a06e.
This reverts valkey-io#792.

Signed-off-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Ping Xie <pingxie@google.com>
PingXie pushed a commit to PingXie/valkey that referenced this pull request Sep 14, 2024
…io#964)

In valkey-io#792, the time complexity became ambiguous, fluctuating between
O(1) and O(n), which is a significant difference. And we agree uncertainty
can potentially bring disaster to the business, the right thing to do is
to persuade users to use EXISTS instead of KEYS in this case, to do the
right thing the right way, rather than accommodating this incorrect usage.

This reverts commit d66a06e.
This reverts valkey-io#792.

Signed-off-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Ping Xie <pingxie@google.com>
PingXie pushed a commit to PingXie/valkey that referenced this pull request Sep 14, 2024
…io#964)

In valkey-io#792, the time complexity became ambiguous, fluctuating between
O(1) and O(n), which is a significant difference. And we agree uncertainty
can potentially bring disaster to the business, the right thing to do is
to persuade users to use EXISTS instead of KEYS in this case, to do the
right thing the right way, rather than accommodating this incorrect usage.

This reverts commit d66a06e.
This reverts valkey-io#792.

Signed-off-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Ping Xie <pingxie@google.com>
PingXie pushed a commit to PingXie/valkey that referenced this pull request Sep 14, 2024
…io#964)

In valkey-io#792, the time complexity became ambiguous, fluctuating between
O(1) and O(n), which is a significant difference. And we agree uncertainty
can potentially bring disaster to the business, the right thing to do is
to persuade users to use EXISTS instead of KEYS in this case, to do the
right thing the right way, rather than accommodating this incorrect usage.

This reverts commit d66a06e.
This reverts valkey-io#792.

Signed-off-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Ping Xie <pingxie@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants