sql: hide experimental_enable_hash_sharded_indexes cluster/session setting#76937
Conversation
|
This change is |
mgartner
left a comment
There was a problem hiding this comment.
🎉
Reviewed 31 of 31 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @chengxiong-ruan and @postamar)
pkg/sql/vars.go, line 1575 at r1 (raw file):
// CockroachDB extension. // This is only kept for backwards compatibility and no longer hash any effect.
AFAIK we remove old settings and let SET fail instead of making it a no-op. Is there a strong reason to differ from that behavior for this particular setting?
|
nit: There's a "sessiong" typo in the commit message and PR title. |
RaduBerinde
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/vars.go, line 1575 at r1 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
AFAIK we remove old settings and let
SETfail instead of making it a no-op. Is there a strong reason to differ from that behavior for this particular setting?
We have one other setting like this (enable_drop_enum_value). I can see the desire to not cause problems for any customers who may have existing scripts that use them; on the other hand, I expect they'd be using the cluster setting, not the session variable.
If we do want to keep it, I would consider separating them out into a "retired variables" list (similar to retired cluster settings) rather than keeping them in this map.
chengxiong-ruan
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/vars.go, line 1575 at r1 (raw file):
Previously, RaduBerinde wrote…
We have one other setting like this (
enable_drop_enum_value). I can see the desire to not cause problems for any customers who may have existing scripts that use them; on the other hand, I expect they'd be using the cluster setting, not the session variable.If we do want to keep it, I would consider separating them out into a "retired variables" list (similar to retired cluster settings) rather than keeping them in this map.
oh, definitely not strong reason to keep it. I agree that removing this and let SET fail would be nice since it send out a strong message that CRDB is getting better :)
Will remove this.
f880a77 to
6c92895
Compare
…tting Release note (sql change): The experimental_enable_hash_sharded_indexes session variable has been removed, along with the corresponding cluster setting. The functionality of being able to create hash sharded index is enabled automatically. SQL statements refer to the setting will still work but will have no effects.
6c92895 to
784ba19
Compare
|
Hooray, we're there! Congratulations to everyone involved. |
|
TFTR! |
|
bors r+ |
|
Build failed (retrying...): |
|
Build succeeded: |
closes #75442
Release note (sql change): The experimental_enable_hash_sharded_indexes
session variable has been removed, along with the corresponding cluster
setting. The functionality of being able to create hash sharded index
is enabled automatically. SQL statements refer to the setting will still
work but will have no effects.