sql: setting to disable NotVisible index feature by wenyihu6 · Pull Request #86033 · cockroachdb/cockroach

wenyihu6 · 2022-08-12T11:52:58Z

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: #72576

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.

cockroach-teamcity · 2022-08-12T11:53:09Z

This change is

michae2

Very nice.

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

michae2 · 2022-08-12T19:11:57Z

Looks like a couple more logic tests will need updating.

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 · 2022-08-13T18:31:38Z

TFTR!!

bors r+

craig · 2022-08-13T19:10:06Z

Build failed (retrying...):

Bazel Essential CI (Cockroach)

craig · 2022-08-13T22:25:06Z

Build succeeded:

Bazel Essential CI (Cockroach)

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.

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

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 <xiang@cockroachlabs.com> Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com> Co-authored-by: Michael Erickson <michae2@cockroachlabs.com> Co-authored-by: Matthew Todd <todd@cockroachlabs.com> Co-authored-by: DrewKimball <drewk@cockroachlabs.com> Co-authored-by: Marylia Gutierrez <marylia@cockroachlabs.com> Co-authored-by: richardjcai <caioftherichard@gmail.com>

wenyihu6 requested a review from a team August 12, 2022 11:52

wenyihu6 requested review from a team as code owners August 12, 2022 11:52

wenyihu6 requested review from a team August 12, 2022 11:52

wenyihu6 requested a review from a team as a code owner August 12, 2022 11:52

wenyihu6 force-pushed the 6-setting-var branch 2 times, most recently from 1b0ba0c to a04fca1 Compare August 12, 2022 11:57

wenyihu6 requested a review from michae2 August 12, 2022 11:57

michae2 approved these changes Aug 12, 2022

View reviewed changes

wenyihu6 force-pushed the 6-setting-var branch 2 times, most recently from 8270a6c to 2fccaf3 Compare August 13, 2022 18:30

wenyihu6 force-pushed the 6-setting-var branch from 2fccaf3 to 5830d70 Compare August 13, 2022 18:30

craig bot merged commit dbce10c into cockroachdb:master Aug 13, 2022

cockroach-teamcity mentioned this pull request Aug 13, 2022

sql: setting to disable NotVisible index feature cockroachdb/docs#14819

Closed

michae2 mentioned this pull request Aug 23, 2022

sql: remove sql.defaults.optimizer_use_not_visible_indexes.enabled #86634

Merged

wenyihu6 deleted the 6-setting-var branch September 1, 2022 20:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sql: setting to disable NotVisible index feature#86033

sql: setting to disable NotVisible index feature#86033
craig[bot] merged 1 commit intocockroachdb:masterfrom
wenyihu6:6-setting-var

wenyihu6 commented Aug 12, 2022 •

edited

Loading

Uh oh!

cockroach-teamcity commented Aug 12, 2022

Uh oh!

michae2 left a comment

Uh oh!

michae2 commented Aug 12, 2022

Uh oh!

wenyihu6 commented Aug 13, 2022

Uh oh!

craig bot commented Aug 13, 2022

Uh oh!

craig bot commented Aug 13, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

wenyihu6 commented Aug 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cockroach-teamcity commented Aug 12, 2022

Uh oh!

michae2 left a comment

Choose a reason for hiding this comment

Uh oh!

michae2 commented Aug 12, 2022

Uh oh!

wenyihu6 commented Aug 13, 2022

Uh oh!

craig bot commented Aug 13, 2022

Uh oh!

craig bot commented Aug 13, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wenyihu6 commented Aug 12, 2022 •

edited

Loading