sql: setting to disable NotVisible index feature #86033
Merged
+132
−16
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
|
This change is |
wenyihu6
force-pushed
the
6-setting-var
branch
2 times, most recently
from
1b0ba0c to
a04fca1
Compare
michae2
approved these changes
Aug 12, 2022
There was a problem hiding this comment.
Reviewed 15 of 15 files at r4, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @michae2 and @wenyihu6)
pkg/sql/exec_util.go line 401 at r4 (raw file):
settings.TenantWritable, "sql.defaults.optimizer_use_not_visible_indexes.enabled", "default value for optimizer_use_not_visible_indexes session setting; disable usage of not visible indexes in the optimizer by default",
nit: Wrap this to 100 chars using + e.g.:
"foo bar " +
"baz",
pkg/sql/opt/xform/scan_index_iter.go line 231 at r4 (raw file):
// If we are not forcing any specific index and not visible index feature is // enabled here, ignore not visible indexes. if index.IsNotVisible() && !it.scanPrivate.Flags.DisableNotVisibleIndex && !it.evalCtx.SessionData().OptimizerUseNotVisibleIndexes {
nit: wrap this to 100 chars by putting the condition after && on the next line
|
Looks like a couple more logic tests will need updating. |
wenyihu6
force-pushed
the
6-setting-var
branch
2 times, most recently
from
8270a6c to
2fccaf3
Compare
Prior to this commit, not visible indexes were always ignored by the optimizer unless it is explicitly selected with index hinting or used for constraint check. This commit adds a session variable along with a cluster setting to control whether the optimizer can still choose to use not visible indexes for the query plan. Note that even if the the session variable is enabled and optimizer can choose to use not visible indexes, not visible indexes remain as not visible in index descriptors. Cluster setting: `sql.defaults.optimizer_use_not_visible_indexes.enabled` Session setting: `optimizer_use_not_visible_indexes` By default, the setting is disabled, meaning not visible index will be properly ignored. When the setting is enabled, optimizer treats not visible indexes as they are visible and can choose to use them for query plan. Assists: cockroachdb#82363 Release note (sql change): Session setting `optimizer_use_not_visible_indexes` and cluster setting `sql.defaults.optimizer_use_not_visible_indexes.enabled` can be used to disable not visible index feature. When they are enabled, optimizer treats not visible indexes as they are visible and can choose to use them for query plan. By default, they are disabled.
wenyihu6
force-pushed
the
6-setting-var
branch
from
2fccaf3 to
5830d70
Compare
|
TFTR!! bors r+ |
|
Build failed (retrying...): |
|
Build succeeded: |
michae2
added a commit
to michae2/cockroach
that referenced
this pull request
Aug 23, 2022
In 22.2 the use of cluster settings for session setting defaults is deprecated, but I forgot this when reviewing cockroachdb#86033. Remove the unreleased default cluster setting we added. Release note (sql change): Remove unreleased cluster setting `sql.defaults.optimizer_use_not_visible_indexes.enabled`. In 22.2 the proper way to set the default value for session settings is to use `ALTER ROLE ALL SET` instead of special cluster settings.
michae2
added a commit
to michae2/cockroach
that referenced
this pull request
Aug 23, 2022
In 22.2 the use of cluster settings for session setting defaults is deprecated, but I forgot this when reviewing cockroachdb#86033. Remove the unreleased default cluster setting we added. Release note: None
michae2
added a commit
to michae2/cockroach
that referenced
this pull request
Aug 24, 2022
In 22.2 the use of cluster settings for session setting defaults is deprecated, but I forgot this when reviewing cockroachdb#86033. Remove the unreleased default cluster setting we added. Release note: None
craig bot
pushed a commit
that referenced
this pull request
Aug 24, 2022
86604: scbuildstmt: added cluster version gating for declarative schema changer r=Xiang-Gu a=Xiang-Gu
For each implemented DDL stmt in the new schema changer, we added a
minimal supported cluster version so that we can return an unimplemented
error when the cluster version is lower than the minimal supported
cluster version of that DDL statement.
This will be useful in mixed-version cluster where we make sure we don't
use new schema changer for statements that are only supported in v22.2,
but not in v22.1.
Partially fix #79840
Release justification: improve safety and prevent issues in
mixed-version cluster.
Release note: None
86605: builtins: don't panic on placeholders in bounded staleness function r=rafiss a=rafiss
fixes #86243
Release note (bug fix): Fixed a crash/panic that could occur if
placeholder arguments were used with the with_min_timestamp or
with_max_staleness functions.
Release justification: Fix a crash caused by a panic.
86607: sql: add nil guard when formatting placeholders r=rafiss a=rafiss
Fixes #85363
Release note (bug fix): Fixed a crash that could happen when formatting
queries that have placeholder BitArray arguments.
Release justification: Fix a panic.
86634: sql: remove sql.defaults.optimizer_use_not_visible_indexes.enabled r=mgartner,wenyihu6,rafiss a=michae2
**sql: remove sql.defaults.optimizer_use_not_visible_indexes.enabled**
In 22.2 the use of cluster settings for session setting defaults is
deprecated, but I forgot this when reviewing #86033. Remove the
unreleased default cluster setting we added.
Release note: None
**sql: add more session variables to statement bundles**
Add some missing session variables to statement diagnostic bundles. This
isn't quite everything, but it's better than what we had before.
Release note: None
----
Release justification: low-risk bug fix.
86673: insights: enable the anomaly detector r=matthewtodd a=matthewtodd
This change turns on the insights "anomaly detector" by default,
catching statement executions in the >p99 latency for their fingerprint
that are also far enough away from median latency and above a (default
50ms) threshold.
Release justification: Category 2: Bug fixes and low-risk updates to new
functionality.
Release note (ops change): The `sql.insights.anomaly_detection.enabled`
cluster setting now defaults to true, and the
`sql.insights.anomaly_detection.latency_threshold` cluster setting now
defaults to 50ms, down from 100ms to complement the fixed-threshold
detector's default of 100ms.
86722: opt: don't mark st_makeline and st_extend as non-null r=DrewKimball a=DrewKimball
`st_makeline` can take arguments of type `POINT`, `MULTIPOINT`, and
`LINESTRING`. Other types cause it to return null (even if the input is
non-null). Similarly, `st_extent` returns null when the input geometry is
non-null, but empty.
This commit prevents these two aggregate functions from being marked as
non-null, since doing so can lead to incorrect results in the cases
mentioned above.
Fixes #84957
Release justification: low-risk fix to bug that can lead to incorrect results
Release note (bug fix): Fixed a longstanding bug that could cause the optimizer
to produce an incorrect plan when aggregate functions `st_makeline` or
`st_extent` were called with invalid-type and empty inputs respectively.
86747: ui: fix typo r=maryliag a=maryliag
Fix typo on Execution Index Recommendation text,
using singular or plural accordingly.
Release justification: low risk change
Release note: None
86791: sql: slight refactor to audit logging code r=rafiss a=RichardJCai
Release justification: Small refactor, no functionality change
Release note: None
Fixes #84760
Co-authored-by: Xiang Gu <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Michael Erickson <[email protected]>
Co-authored-by: Matthew Todd <[email protected]>
Co-authored-by: DrewKimball <[email protected]>
Co-authored-by: Marylia Gutierrez <[email protected]>
Co-authored-by: richardjcai <[email protected]>
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.
Prior to this commit, not visible indexes were always ignored by the optimizer
unless it is explicitly selected with index hinting or used for constraint
check. This commit adds a session variable along with a cluster setting to
control whether the optimizer can still choose to use not visible indexes for
the query plan.
Note that even if the the session variable is enabled and optimizer can choose
to use not visible indexes, not visible indexes remain as not visible in index
descriptors.
Cluster setting:
sql.defaults.optimizer_use_not_visible_indexes.enabledSession setting:
optimizer_use_not_visible_indexesBy default, the setting isdisabled, meaning not visible index will be properly ignored. When the setting
is enabled, optimizer treats not visible indexes as they are visible and can
choose to use them for query plan.
Assists: #72576
Release note (sql change): Session setting
optimizer_use_not_visible_indexesand cluster setting
sql.defaults.optimizer_use_not_visible_indexes.enabledcanbe used to disable not visible index feature. When they are enabled, optimizer
treats not visible indexes as they are visible and can choose to use them for
query plan. By default, they are disabled.