From 4c7d5bd080a7987bcde3d998dcdb105062624b9d Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Sun, 19 Mar 2023 14:03:41 +0100 Subject: [PATCH 1/2] sql: introduce planning error hints This change introduces a new stage during planning, which upon encountering a planning error adds additional user-facing hints to the error payload. We will be able to extend this over time to make suggestions on how to enhance a query to avoid error. As an example application, this change enhances errors about now-removed columns in `crdb_internal.ranges`, `ranges_no_leases` and the output of `SHOW RANGES`. For example: ``` demo@127.0.0.1:26257/movr> select lease_holder from [show ranges from table users]; ERROR: column "lease_holder" does not exist SQLSTATE: 42703 HINT: To list lease holder and range size details, consider SHOW RANGES WITH DETAILS. -- There are more SHOW RANGES options. Refer to the online documentation or execute 'SHOW RANGES ??' for details. ``` ``` demo@127.0.0.1:26257/movr> select database_name,table_name from crdb_internal.ranges; ERROR: column "database_name" does not exist SQLSTATE: 42703 DETAIL: SELECT database_name, table_name FROM crdb_internal.ranges HINT: To list all ranges across all databases and display the database name, consider SHOW CLUSTER RANGES WITH TABLES. -- There are more SHOW RANGES options. Refer to the online documentation or execute 'SHOW RANGES ??' for details. ``` Release note: None --- pkg/sql/BUILD.bazel | 1 + pkg/sql/conn_executor_exec.go | 3 +- pkg/sql/error_hints.go | 72 +++++++++++++++++++ .../logictest/testdata/logic_test/show_ranges | 38 ++++++++++ 4 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 pkg/sql/error_hints.go diff --git a/pkg/sql/BUILD.bazel b/pkg/sql/BUILD.bazel index f5e0d660f4d6..2979625683bc 100644 --- a/pkg/sql/BUILD.bazel +++ b/pkg/sql/BUILD.bazel @@ -105,6 +105,7 @@ go_library( "drop_tenant.go", "drop_type.go", "drop_view.go", + "error_hints.go", "error_if_rows.go", "event_log.go", "exec_factory_util.go", diff --git a/pkg/sql/conn_executor_exec.go b/pkg/sql/conn_executor_exec.go index c5be432c8513..354882a3e232 100644 --- a/pkg/sql/conn_executor_exec.go +++ b/pkg/sql/conn_executor_exec.go @@ -1246,7 +1246,7 @@ func (ex *connExecutor) dispatchToExecutionEngine( err := ex.makeExecPlan(ctx, planner) defer planner.curPlan.close(ctx) - // include gist in error reports + // Include gist in error reports. ctx = withPlanGist(ctx, planner.instrumentation.planGist.String()) if planner.extendedEvalCtx.TxnImplicit { planner.curPlan.flags.Set(planFlagImplicitTxn) @@ -1278,6 +1278,7 @@ func (ex *connExecutor) dispatchToExecutionEngine( // Finally, process the planning error from above. if err != nil { + err = addPlanningErrorHints(err, &stmt) res.SetError(err) return nil } diff --git a/pkg/sql/error_hints.go b/pkg/sql/error_hints.go new file mode 100644 index 000000000000..1306fd165c4e --- /dev/null +++ b/pkg/sql/error_hints.go @@ -0,0 +1,72 @@ +// Copyright 2023 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package sql + +import ( + "strings" + + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + "github.com/cockroachdb/errors" +) + +// addPlanningErrorHints is responsible for enhancing the provided +// error object with hints that inform the user about their options. +func addPlanningErrorHints(err error, stmt *Statement) error { + pgCode := pgerror.GetPGCode(err) + switch pgCode { + case pgcode.UndefinedColumn: + resolvedSQL := stmt.AST.String() + + // Changes introduced in v23.1. + extraRangeDoc := false + switch { + case strings.Contains(resolvedSQL, "SHOW RANGES"): + errS := err.Error() + + // The following columns are not available when using SHOW + // RANGES after the introduction of coalesced ranges. + if strings.Contains(errS, "lease_holder") || strings.Contains(errS, "range_size") { + err = errors.WithHint(err, + "To list lease holder and range size details, consider SHOW RANGES WITH DETAILS.") + extraRangeDoc = true + } + + case strings.Contains(resolvedSQL, "crdb_internal.ranges" /* also matches ranges_no_leases */): + errS := err.Error() + + // The following columns are not available when using SHOW + // RANGES after the introduction of coalesced ranges. + if strings.Contains(errS, "database_name") { + err = errors.WithHint(err, + "To list all ranges across all databases and display the database name, consider SHOW CLUSTER RANGES WITH TABLES.") + extraRangeDoc = true + } + if strings.Contains(errS, "schema_name") || + strings.Contains(errS, "table_name") || + strings.Contains(errS, "table_id") { + err = errors.WithHint(err, + "To retrieve table/schema names/IDs associated with ranges, consider SHOW [CLUSTER] RANGES WITH TABLES.") + extraRangeDoc = true + } + if strings.Contains(errS, "index_name") { + err = errors.WithHint(err, + "To retrieve index names associated with ranges, consider SHOW [CLUSTER] RANGES WITH INDEXES.") + extraRangeDoc = true + } + } + if extraRangeDoc { + err = errors.WithHint(err, + "There are more SHOW RANGES options. Refer to the online documentation or execute 'SHOW RANGES ??' for details.") + } + } + return err +} diff --git a/pkg/sql/logictest/testdata/logic_test/show_ranges b/pkg/sql/logictest/testdata/logic_test/show_ranges index 94347c450660..e02d07370102 100644 --- a/pkg/sql/logictest/testdata/logic_test/show_ranges +++ b/pkg/sql/logictest/testdata/logic_test/show_ranges @@ -454,3 +454,41 @@ CREATE TABLE v0 (c1 BIT PRIMARY KEY ); statement error pgcode 42846 pq: crdb_internal.encode_key\(\): invalid cast: bytes -> bit SHOW RANGE FROM TABLE v0 FOR ROW ( b'\x68') + +subtest prev_interface_error_hints + +statement error pgcode 42703 .*\nHINT.*lease holder and range size +SELECT lease_holder FROM [SHOW RANGES FROM TABLE v0] + +statement error pgcode 42703 .*\nHINT.*lease holder and range size +SELECT lease_holder FROM [show ranges from table v0] + +statement error pgcode 42703 .*\nHINT.*lease holder and range size +SELECT lease_holder_locality FROM [SHOW RANGES FROM TABLE v0] + +statement error pgcode 42703 .*\nHINT.*lease holder and range size +SELECT range_size FROM [SHOW RANGES FROM TABLE v0] + +statement error pgcode 42703 .*\nHINT.*lease holder and range size +SELECT range_size_mb FROM [SHOW RANGES FROM TABLE v0] + +statement error pgcode 42703 .*\nHINT.*display the database name +SELECT database_name FROM crdb_internal.ranges + +statement error pgcode 42703 .*\nHINT.*display the database name +SELECT database_name FROM CRDB_INTERNAL."ranges" + +statement error pgcode 42703 .*\nHINT.*display the database name +SELECT database_name FROM crdb_internal.ranges_no_leases + +statement error pgcode 42703 .*\nHINT.*To retrieve table/schema.*with ranges +SELECT table_name FROM crdb_internal.ranges + +statement error pgcode 42703 .*\nHINT.*To retrieve table/schema.*with ranges +SELECT table_id FROM crdb_internal.ranges + +statement error pgcode 42703 .*\nHINT.*To retrieve table/schema.*with ranges +SELECT schema_name FROM crdb_internal.ranges + +statement error pgcode 42703 .*\nHINT.*To retrieve index.*with ranges +SELECT index_name FROM crdb_internal.ranges From 87afa373c435919ec3ae3e2860a977403c78d562 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Mon, 20 Mar 2023 23:50:15 +0100 Subject: [PATCH 2/2] sql: backward-compat for `SHOW RANGES` / `crdb_internal.ranges{,_no_leases}` TLDR: the pre-v23.1 behavior of SHOW RANGES and `crdb_internal.ranges{_no_leases}` is made available conditional on a new cluster setting `sql.show_ranges_deprecated_behavior.enabled`. It is set to true by default in v23.1, however any use of the feature will prompt a SQL notice with the following message: ``` NOTICE: attention! the pre-23.1 behavior of SHOW RANGES and crdb_internal.ranges{,_no_leases} is deprecated! HINT: Consider enabling the new functionality by setting 'sql.show_ranges_deprecated_behavior.enabled' to 'false'. The new SHOW RANGES statement has more options. Refer to the online documentation or execute 'SHOW RANGES ??' for details. ``` The deprecated behavior is also disabled in the test suite and in `cockroach demo`, i.e. the tests continue to exercise the new behavior. cf. doc for new package `deprecatedshowranges`: ``` // Package deprecatedshowranges exists to smoothen the transition // between the pre-v23.1 semantics of SHOW RANGES and // crdb_internal.ranges{_no_leases}, and the new semantics introduced // in v23.1. // // The pre-v23.1 semantics are deprecated as of v23.1. At the end of // the deprecation cycle (hopefully for v23.2) we expect to delete // this package entirely and all the other code in the SQL layer that // hangs off the EnableDeprecatedBehavior() conditional defined below. // // The mechanism to control the behavior is as follows: // // - In any case, an operator can override the behavior using an env // var, "COCKROACH_FORCE_DEPRECATED_SHOW_RANGE_BEHAVIOR". // If set (to a boolean), the value of the env var is used in // all cases. // We use this env var to force the _new_ behavior through out // test suite, regardless of the other conditions. This allows // us to avoid maintaining two sets of outputs in tests. // // - If the env var is not set, the cluster setting // `sql.show_ranges_deprecated_behavior.enabled` is used. It // defaults to true in v23.1 (the "smoothening" part). // // - If the deprecated behavior is chosen by any of the above // mechanisms, and _the range coalescing cluster setting_ is set // to true, a loud warning will be reported in various places. // This will nudge users who wish to opt into range coalescing // to adapt their use of the range inspection accordingly. ``` Release note (backward-incompatible change): The pre-v23.1 output produced by `SHOW RANGES`, `crdb_internal.ranges`, `crdb_internal.ranges_no_leases` is deprecated. The new interface and functionality for `SHOW RANGES` can be enabled by changing the cluster setting `sql.show_ranges_deprecated_behavior.enabled` to `false`. It will become default in v23.2. --- .../settings/settings-for-tenants.txt | 1 + docs/generated/settings/settings.html | 1 + pkg/BUILD.bazel | 2 + pkg/acceptance/cluster/dockercluster.go | 3 + pkg/cmd/roachtest/cluster.go | 8 + pkg/server/BUILD.bazel | 1 + pkg/server/admin.go | 53 +++- pkg/server/settingswatcher/BUILD.bazel | 1 + .../settings_watcher_external_test.go | 17 ++ pkg/server/testserver.go | 16 ++ pkg/spanconfig/spanconfigstore/span_store.go | 6 +- pkg/sql/BUILD.bazel | 2 + pkg/sql/conn_executor.go | 6 +- pkg/sql/conn_executor_exec.go | 2 +- pkg/sql/conn_executor_test.go | 2 +- pkg/sql/crdb_internal.go | 2 +- pkg/sql/crdb_internal_ranges_deprecated.go | 244 ++++++++++++++++++ pkg/sql/delegate/BUILD.bazel | 2 + pkg/sql/delegate/delegate.go | 8 + pkg/sql/delegate/show_ranges_deprecated.go | 143 ++++++++++ pkg/sql/deprecatedshowranges/BUILD.bazel | 21 ++ pkg/sql/deprecatedshowranges/condition.go | 122 +++++++++ pkg/sql/error_hints.go | 13 +- pkg/sql/event_log.go | 2 +- pkg/sql/exec_factory_util.go | 2 +- pkg/sql/exec_util.go | 30 ++- pkg/sql/exec_util_test.go | 2 +- pkg/sql/opt_exec_factory.go | 2 +- pkg/sql/pg_catalog.go | 12 +- pkg/sql/schema_change_plan_node.go | 2 +- pkg/sql/statement_mark_redaction_test.go | 2 +- pkg/sql/virtual_schema.go | 107 +++++++- 32 files changed, 787 insertions(+), 50 deletions(-) create mode 100644 pkg/sql/crdb_internal_ranges_deprecated.go create mode 100644 pkg/sql/delegate/show_ranges_deprecated.go create mode 100644 pkg/sql/deprecatedshowranges/BUILD.bazel create mode 100644 pkg/sql/deprecatedshowranges/condition.go diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index c54730fd8deb..d9177334637d 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -251,6 +251,7 @@ sql.notices.enabled boolean true enable notices in the server/client protocol be sql.optimizer.uniqueness_checks_for_gen_random_uuid.enabled boolean false if enabled, uniqueness checks may be planned for mutations of UUID columns updated with gen_random_uuid(); otherwise, uniqueness is assumed due to near-zero collision probability sql.pgwire.multiple_active_portals.enabled boolean false if true, portals with read-only SELECT query without sub/post queries can be executed in interleaving manner, but with local execution plan sql.schema.telemetry.recurrence string @weekly cron-tab recurrence for SQL schema telemetry job +sql.show_ranges_deprecated_behavior.enabled boolean true if set, SHOW RANGES and crdb_internal.ranges{_no_leases} behave with deprecated pre-v23.1 semantics. NB: the new SHOW RANGES interface has richer WITH options than pre-v23.1 SHOW RANGES. sql.spatial.experimental_box2d_comparison_operators.enabled boolean false enables the use of certain experimental box2d comparison operators sql.stats.automatic_collection.enabled boolean true automatic statistics collection mode sql.stats.automatic_collection.fraction_stale_rows float 0.2 target fraction of stale rows per table that will trigger a statistics refresh diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index b2d6a68d2e64..29f55e6e732b 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -202,6 +202,7 @@
sql.optimizer.uniqueness_checks_for_gen_random_uuid.enabled
booleanfalseif enabled, uniqueness checks may be planned for mutations of UUID columns updated with gen_random_uuid(); otherwise, uniqueness is assumed due to near-zero collision probability
sql.pgwire.multiple_active_portals.enabled
booleanfalseif true, portals with read-only SELECT query without sub/post queries can be executed in interleaving manner, but with local execution plan
sql.schema.telemetry.recurrence
string@weeklycron-tab recurrence for SQL schema telemetry job +
sql.show_ranges_deprecated_behavior.enabled
booleantrueif set, SHOW RANGES and crdb_internal.ranges{_no_leases} behave with deprecated pre-v23.1 semantics. NB: the new SHOW RANGES interface has richer WITH options than pre-v23.1 SHOW RANGES.
sql.spatial.experimental_box2d_comparison_operators.enabled
booleanfalseenables the use of certain experimental box2d comparison operators
sql.stats.automatic_collection.enabled
booleantrueautomatic statistics collection mode
sql.stats.automatic_collection.fraction_stale_rows
float0.2target fraction of stale rows per table that will trigger a statistics refresh diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 10a4f6eb4b20..3fb0c7bc40a6 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -1641,6 +1641,7 @@ GO_TARGETS = [ "//pkg/sql/decodeusername:decodeusername", "//pkg/sql/decodeusername:decodeusername_test", "//pkg/sql/delegate:delegate", + "//pkg/sql/deprecatedshowranges:deprecatedshowranges", "//pkg/sql/descmetadata:descmetadata", "//pkg/sql/descmetadata:descmetadata_test", "//pkg/sql/distsql:distsql", @@ -2924,6 +2925,7 @@ GET_X_DATA_TARGETS = [ "//pkg/sql/covering:get_x_data", "//pkg/sql/decodeusername:get_x_data", "//pkg/sql/delegate:get_x_data", + "//pkg/sql/deprecatedshowranges:get_x_data", "//pkg/sql/descmetadata:get_x_data", "//pkg/sql/distsql:get_x_data", "//pkg/sql/doctor:get_x_data", diff --git a/pkg/acceptance/cluster/dockercluster.go b/pkg/acceptance/cluster/dockercluster.go index d8eb981ec413..d3f74fced8b6 100644 --- a/pkg/acceptance/cluster/dockercluster.go +++ b/pkg/acceptance/cluster/dockercluster.go @@ -493,6 +493,9 @@ func (l *DockerCluster) startNode(ctx context.Context, node *testNode) { "COCKROACH_SCAN_MAX_IDLE_TIME=200ms", "COCKROACH_SKIP_UPDATE_CHECK=1", "COCKROACH_CRASH_REPORTS=", + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + "COCKROACH_FORCE_DEPRECATED_SHOW_RANGE_BEHAVIOR=false", } l.createRoach(ctx, node, l.vols, env, cmd...) maybePanic(node.Start(ctx)) diff --git a/pkg/cmd/roachtest/cluster.go b/pkg/cmd/roachtest/cluster.go index 2916ad0d87c8..a14a144d2e1e 100644 --- a/pkg/cmd/roachtest/cluster.go +++ b/pkg/cmd/roachtest/cluster.go @@ -1837,6 +1837,14 @@ func (c *clusterImpl) StartE( settings.Env = append(settings.Env, "COCKROACH_CRASH_ON_SPAN_USE_AFTER_FINISH=true") } + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + if !envExists(settings.Env, "COCKROACH_FORCE_DEPRECATED_SHOW_RANGE_BEHAVIOR") { + // This makes all roachtest use the new SHOW RANGES behavior, + // regardless of cluster settings. + settings.Env = append(settings.Env, "COCKROACH_FORCE_DEPRECATED_SHOW_RANGE_BEHAVIOR=false") + } + clusterSettingsOpts := []install.ClusterSettingOption{ install.TagOption(settings.Tag), install.PGUrlCertsDirOption(settings.PGUrlCertsDir), diff --git a/pkg/server/BUILD.bazel b/pkg/server/BUILD.bazel index 1a8fba4da9f1..61145a5a385e 100644 --- a/pkg/server/BUILD.bazel +++ b/pkg/server/BUILD.bazel @@ -208,6 +208,7 @@ go_library( "//pkg/sql/consistencychecker", "//pkg/sql/contention", "//pkg/sql/contentionpb", + "//pkg/sql/deprecatedshowranges", "//pkg/sql/distsql", "//pkg/sql/execinfra", "//pkg/sql/execinfrapb", diff --git a/pkg/server/admin.go b/pkg/server/admin.go index cb29d6144129..2b61acd3a428 100644 --- a/pkg/server/admin.go +++ b/pkg/server/admin.go @@ -48,6 +48,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/deprecatedshowranges" "github.com/cockroachdb/cockroach/pkg/sql/isql" "github.com/cockroachdb/cockroach/pkg/sql/optionalnodeliveness" "github.com/cockroachdb/cockroach/pkg/sql/parser" @@ -1054,11 +1055,50 @@ func (s *adminServer) tableDetailsHelper( } // MVCC Garbage result. - row, cols, err = s.internalExecutor.QueryRowExWithCols( - ctx, "admin-show-mvcc-garbage-info", nil, - sessiondata.InternalExecutorOverride{User: userName}, - fmt.Sprintf( + + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + if deprecatedshowranges.ShowRangesDeprecatedBehaviorSetting.Get(&s.st.SV) { + // NOTE: this query is deprecated. See the "else" branch below for + // the current code. + // + // This query is unable to handle table names with a schema qualification. + // If a user complains about this, tell them to opt out of the deprecated + // SHOW RANGES behavior, which will enable the proper semantics below. + tbName := strings.TrimPrefix(req.Table, "public.") + row, cols, err = s.internalExecutor.QueryRowExWithCols( + ctx, "admin-show-mvcc-garbage-info", nil, + sessiondata.InternalExecutorOverride{User: userName}, `WITH + range_stats AS ( + SELECT crdb_internal.range_stats(start_key) AS d + FROM crdb_internal.ranges_no_leases WHERE database_name = $1 AND table_name = $2 + ), + aggregated AS ( + SELECT + sum((d->>'live_bytes')::INT8) AS live, + sum( + (d->>'key_bytes')::INT8 + + (d->>'val_bytes')::INT8 + + COALESCE((d->>'range_key_bytes')::INT8, 0) + + COALESCE((d->>'range_val_bytes')::INT8, 0) + + (d->>'sys_bytes')::INT8) AS total + FROM + range_stats + ) + SELECT + COALESCE(total, 0)::INT8 as total_bytes, + COALESCE(live, 0)::INT8 as live_bytes, + COALESCE(live / NULLIF(total,0), 0)::FLOAT8 as live_percentage + FROM aggregated`, + req.Database, tbName, + ) + } else { + row, cols, err = s.internalExecutor.QueryRowExWithCols( + ctx, "admin-show-mvcc-garbage-info", nil, + sessiondata.InternalExecutorOverride{User: userName}, + fmt.Sprintf( + `WITH range_stats AS ( SELECT crdb_internal.range_stats(raw_start_key) AS d FROM [SHOW RANGES FROM TABLE %s WITH KEYS] @@ -1080,8 +1120,9 @@ func (s *adminServer) tableDetailsHelper( COALESCE(live, 0)::INT8 as live_bytes, COALESCE(live / NULLIF(total,0), 0)::FLOAT8 as live_percentage FROM aggregated`, - escQualTable), - ) + escQualTable), + ) + } if err != nil { return nil, err } diff --git a/pkg/server/settingswatcher/BUILD.bazel b/pkg/server/settingswatcher/BUILD.bazel index 94f8d4be95e9..9fac911f3d76 100644 --- a/pkg/server/settingswatcher/BUILD.bazel +++ b/pkg/server/settingswatcher/BUILD.bazel @@ -65,6 +65,7 @@ go_test( "//pkg/settings", "//pkg/settings/cluster", "//pkg/sql", + "//pkg/sql/deprecatedshowranges", "//pkg/sql/sem/tree", "//pkg/testutils", "//pkg/testutils/serverutils", diff --git a/pkg/server/settingswatcher/settings_watcher_external_test.go b/pkg/server/settingswatcher/settings_watcher_external_test.go index 56d01984502b..998c4d728517 100644 --- a/pkg/server/settingswatcher/settings_watcher_external_test.go +++ b/pkg/server/settingswatcher/settings_watcher_external_test.go @@ -29,6 +29,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql" + "github.com/cockroachdb/cockroach/pkg/sql/deprecatedshowranges" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" @@ -135,6 +136,14 @@ func TestSettingWatcherOnTenant(t *testing.T) { s0 := tc.Server(0) tenantSettings := cluster.MakeTestingClusterSettings() tenantSettings.SV.SetNonSystemTenant() + + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + deprecatedshowranges.ShowRangesDeprecatedBehaviorSetting.Override( + ctx, &tenantSettings.SV, + // In unit tests, we exercise the new behavior. + false) + storage := &fakeStorage{} sw := settingswatcher.New(s0.Clock(), fakeCodec, tenantSettings, s0.ExecutorConfig().(sql.ExecutorConfig).RangeFeedFactory, @@ -373,6 +382,14 @@ func TestOverflowRestart(t *testing.T) { defer s.Stopper().Stop(ctx) sideSettings := cluster.MakeTestingClusterSettings() + + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + deprecatedshowranges.ShowRangesDeprecatedBehaviorSetting.Override( + ctx, &sideSettings.SV, + // In unit tests, we exercise the new behavior. + false) + w := settingswatcher.New( s.Clock(), s.ExecutorConfig().(sql.ExecutorConfig).Codec, diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go index 98028624d463..0716a5601576 100644 --- a/pkg/server/testserver.go +++ b/pkg/server/testserver.go @@ -45,6 +45,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/catalog/bootstrap" + "github.com/cockroachdb/cockroach/pkg/sql/deprecatedshowranges" "github.com/cockroachdb/cockroach/pkg/sql/pgwire" "github.com/cockroachdb/cockroach/pkg/sql/physicalplan" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" @@ -137,6 +138,14 @@ func makeTestConfigFromParams(params base.TestServerArgs) Config { if params.Settings == nil { st = cluster.MakeClusterSettings() } + + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + deprecatedshowranges.ShowRangesDeprecatedBehaviorSetting.Override( + context.TODO(), &st.SV, + // In unit tests, we exercise the new behavior. + false) + st.ExternalIODir = params.ExternalIODir tr := params.Tracer if params.Tracer == nil { @@ -1001,6 +1010,13 @@ func (ts *TestServer) StartTenant( st = cluster.MakeTestingClusterSettings() } + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + deprecatedshowranges.ShowRangesDeprecatedBehaviorSetting.Override( + context.TODO(), &st.SV, + // In unit tests, we exercise the new behavior. + false) + st.ExternalIODir = params.ExternalIODir sqlCfg := makeTestSQLConfig(st, params.TenantID) sqlCfg.TenantKVAddrs = []string{ts.ServingRPCAddr()} diff --git a/pkg/spanconfig/spanconfigstore/span_store.go b/pkg/spanconfig/spanconfigstore/span_store.go index 269da4e8379f..e1a9f2833446 100644 --- a/pkg/spanconfig/spanconfigstore/span_store.go +++ b/pkg/spanconfig/spanconfigstore/span_store.go @@ -34,10 +34,10 @@ var tenantCoalesceAdjacentSetting = settings.RegisterBoolSetting( true, ) -// storageCoalesceAdjacentSetting is a hidden cluster setting that +// StorageCoalesceAdjacentSetting is a hidden cluster setting that // controls whether we coalesce adjacent ranges outside of the // secondary tenant keyspaces if they have the same span config. -var storageCoalesceAdjacentSetting = settings.RegisterBoolSetting( +var StorageCoalesceAdjacentSetting = settings.RegisterBoolSetting( settings.SystemOnly, "spanconfig.storage_coalesce_adjacent.enabled", `collapse adjacent ranges with the same span configs for the ranges specific to the system tenant`, @@ -152,7 +152,7 @@ func (s *spanConfigStore) computeSplitKey(start, end roachpb.RKey) (roachpb.RKey // ranges. systemTableUpperBound := keys.SystemSQLCodec.TablePrefix(keys.MaxReservedDescID + 1) if roachpb.Key(rem).Compare(systemTableUpperBound) < 0 || - !storageCoalesceAdjacentSetting.Get(&s.settings.SV) { + !StorageCoalesceAdjacentSetting.Get(&s.settings.SV) { return roachpb.RKey(match.span.Key), nil } } else { diff --git a/pkg/sql/BUILD.bazel b/pkg/sql/BUILD.bazel index 2979625683bc..dfafaedccefd 100644 --- a/pkg/sql/BUILD.bazel +++ b/pkg/sql/BUILD.bazel @@ -56,6 +56,7 @@ go_library( "copy_from.go", "copy_to.go", "crdb_internal.go", + "crdb_internal_ranges_deprecated.go", "create_database.go", "create_extension.go", "create_external_connection.go", @@ -389,6 +390,7 @@ go_library( "//pkg/sql/covering", "//pkg/sql/decodeusername", "//pkg/sql/delegate", + "//pkg/sql/deprecatedshowranges", "//pkg/sql/descmetadata", "//pkg/sql/distsql", "//pkg/sql/enum", diff --git a/pkg/sql/conn_executor.go b/pkg/sql/conn_executor.go index bba2021e90e0..ef8a0a0721df 100644 --- a/pkg/sql/conn_executor.go +++ b/pkg/sql/conn_executor.go @@ -681,7 +681,7 @@ func (s *Server) getScrubbedStmtStats( var scrubbedStats []appstatspb.CollectedStatementStatistics stmtStatsVisitor := func(_ context.Context, stat *appstatspb.CollectedStatementStatistics) error { // Scrub the statement itself. - scrubbedQueryStr, ok := scrubStmtStatKey(s.cfg.VirtualSchemas, stat.Key.Query) + scrubbedQueryStr, ok := scrubStmtStatKey(s.cfg.VirtualSchemas, stat.Key.Query, nil) // We don't want to report this stats if scrubbing has failed. We also don't // wish to abort here because we want to try our best to report all the @@ -1110,7 +1110,7 @@ func (ex *connExecutor) closeWrapper(ctx context.Context, recovered interface{}) // Embed the statement in the error object for the telemetry // report below. The statement gets anonymized. vt := ex.planner.extendedEvalCtx.VirtualSchemas - panicErr = WithAnonymizedStatement(panicErr, ex.curStmtAST, vt) + panicErr = WithAnonymizedStatement(panicErr, ex.curStmtAST, vt, nil) } // Report the panic to telemetry in any case. @@ -4217,7 +4217,7 @@ func init() { return "" } // Anonymize the statement for reporting. - return anonymizeStmtAndConstants(stmt, nil /* VirtualTabler */) + return anonymizeStmtAndConstants(stmt, nil /* VirtualTabler */, nil /* ClientNoticeSender */) }) logcrash.RegisterTagFn("gist", func(ctx context.Context) string { return planGistFromCtx(ctx) diff --git a/pkg/sql/conn_executor_exec.go b/pkg/sql/conn_executor_exec.go index 354882a3e232..483859ab6392 100644 --- a/pkg/sql/conn_executor_exec.go +++ b/pkg/sql/conn_executor_exec.go @@ -1278,7 +1278,7 @@ func (ex *connExecutor) dispatchToExecutionEngine( // Finally, process the planning error from above. if err != nil { - err = addPlanningErrorHints(err, &stmt) + err = addPlanningErrorHints(ctx, err, &stmt, ex.server.cfg.Settings, planner) res.SetError(err) return nil } diff --git a/pkg/sql/conn_executor_test.go b/pkg/sql/conn_executor_test.go index d70c0af038d1..9464b26341ab 100644 --- a/pkg/sql/conn_executor_test.go +++ b/pkg/sql/conn_executor_test.go @@ -84,7 +84,7 @@ INSERT INTO sensitive(super, sensible) VALUES('that', 'nobody', 'must', 'see') } rUnsafe := errors.New("some error") - safeErr := sql.WithAnonymizedStatement(rUnsafe, stmt1.AST, vt) + safeErr := sql.WithAnonymizedStatement(rUnsafe, stmt1.AST, vt, nil /* ClientNoticeSender */) const expMessage = "some error" actMessage := safeErr.Error() diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go index bb2468142a84..17fc3be4519a 100644 --- a/pkg/sql/crdb_internal.go +++ b/pkg/sql/crdb_internal.go @@ -1502,7 +1502,7 @@ CREATE TABLE crdb_internal.node_statement_statistics ( statementVisitor := func(_ context.Context, stats *appstatspb.CollectedStatementStatistics) error { anonymized := tree.DNull - anonStr, ok := scrubStmtStatKey(p.getVirtualTabler(), stats.Key.Query) + anonStr, ok := scrubStmtStatKey(p.getVirtualTabler(), stats.Key.Query, p) if ok { anonymized = tree.NewDString(anonStr) } diff --git a/pkg/sql/crdb_internal_ranges_deprecated.go b/pkg/sql/crdb_internal_ranges_deprecated.go new file mode 100644 index 000000000000..956a9c112e6d --- /dev/null +++ b/pkg/sql/crdb_internal_ranges_deprecated.go @@ -0,0 +1,244 @@ +// Copyright 2023 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package sql + +import ( + "context" + "sort" + + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + "github.com/cockroachdb/cockroach/pkg/sql/privilege" + "github.com/cockroachdb/cockroach/pkg/sql/sem/eval" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/types" + "github.com/cockroachdb/cockroach/pkg/util/errorutil" + "github.com/cockroachdb/cockroach/pkg/util/stop" +) + +// crdbInternalRangesViewDEPRECATED is the pre-v23.1 +// version of `crdb_internal.ranges`. See the package-level doc +// of package `deprecatedshowranges` for details. +// +// Remove this code in v23.2. +var crdbInternalRangesViewDEPRECATED = virtualSchemaView{ + schema: ` +CREATE VIEW crdb_internal.ranges AS SELECT + range_id, + start_key, + start_pretty, + end_key, + end_pretty, + table_id, + database_name, + schema_name, + table_name, + index_name, + replicas, + replica_localities, + voting_replicas, + non_voting_replicas, + learner_replicas, + split_enforced_until, + crdb_internal.lease_holder(start_key) AS lease_holder, + (crdb_internal.range_stats(start_key)->>'key_bytes')::INT + + (crdb_internal.range_stats(start_key)->>'val_bytes')::INT + + coalesce((crdb_internal.range_stats(start_key)->>'range_key_bytes')::INT, 0) + + coalesce((crdb_internal.range_stats(start_key)->>'range_val_bytes')::INT, 0) AS range_size +FROM crdb_internal.ranges_no_leases +`, + resultColumns: colinfo.ResultColumns{ + {Name: "range_id", Typ: types.Int}, + {Name: "start_key", Typ: types.Bytes}, + {Name: "start_pretty", Typ: types.String}, + {Name: "end_key", Typ: types.Bytes}, + {Name: "end_pretty", Typ: types.String}, + {Name: "table_id", Typ: types.Int}, + {Name: "database_name", Typ: types.String}, + {Name: "schema_name", Typ: types.String}, + {Name: "table_name", Typ: types.String}, + {Name: "index_name", Typ: types.String}, + {Name: "replicas", Typ: types.Int2Vector}, + {Name: "replica_localities", Typ: types.StringArray}, + {Name: "voting_replicas", Typ: types.Int2Vector}, + {Name: "non_voting_replicas", Typ: types.Int2Vector}, + {Name: "learner_replicas", Typ: types.Int2Vector}, + {Name: "split_enforced_until", Typ: types.Timestamp}, + {Name: "lease_holder", Typ: types.Int}, + {Name: "range_size", Typ: types.Int}, + }, +} + +// crdbInternalRangesNoLeasesTableDEPRECATED is the pre-v23.1 version +// of `crdb_internal.ranges_no_leases`. See the package-level doc of +// package `deprecatedshowranges` for details. +// +// Remove this code in v23.2. +var crdbInternalRangesNoLeasesTableDEPRECATED = virtualSchemaTable{ + comment: `range metadata without leaseholder details (KV join; expensive!)`, + // NB 1: The `replicas` column is the union of `voting_replicas` and + // `non_voting_replicas` and does not include `learner_replicas`. + // NB 2: All the values in the `*replicas` columns correspond to store IDs. + schema: ` +CREATE TABLE crdb_internal.ranges_no_leases ( + range_id INT NOT NULL, + start_key BYTES NOT NULL, + start_pretty STRING NOT NULL, + end_key BYTES NOT NULL, + end_pretty STRING NOT NULL, + table_id INT NOT NULL, + database_name STRING NOT NULL, + schema_name STRING NOT NULL, + table_name STRING NOT NULL, + index_name STRING NOT NULL, + replicas INT[] NOT NULL, + replica_localities STRING[] NOT NULL, + voting_replicas INT[] NOT NULL, + non_voting_replicas INT[] NOT NULL, + learner_replicas INT[] NOT NULL, + split_enforced_until TIMESTAMP +) +`, + generator: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, _ *stop.Stopper) (virtualTableGenerator, cleanupFunc, error) { + hasAdmin, err := p.HasAdminRole(ctx) + if err != nil { + return nil, nil, err + } + all, err := p.Descriptors().GetAllDescriptors(ctx, p.txn) + if err != nil { + return nil, nil, err + } + descs := all.OrderedDescriptors() + + privCheckerFunc := func(desc catalog.Descriptor) (bool, error) { + if hasAdmin { + return true, nil + } + + return p.HasPrivilege(ctx, desc, privilege.ZONECONFIG, p.User()) + } + + hasPermission, dbNames, tableNames, schemaNames, indexNames, schemaParents, parents, err := + descriptorsByType(descs, privCheckerFunc) + if err != nil { + return nil, nil, err + } + + // if the user has no ZONECONFIG privilege on any table/schema/database + if !hasPermission { + return nil, nil, pgerror.Newf(pgcode.InsufficientPrivilege, "only users with the ZONECONFIG privilege or the admin role can read crdb_internal.ranges_no_leases") + } + + execCfg := p.ExecCfg() + rangeDescIterator, err := execCfg.RangeDescIteratorFactory.NewIterator(ctx, execCfg.Codec.TenantSpan()) + if err != nil { + return nil, nil, err + } + + return func() (tree.Datums, error) { + if !rangeDescIterator.Valid() { + return nil, nil + } + + rangeDesc := rangeDescIterator.CurRangeDescriptor() + + rangeDescIterator.Next() + + replicas := rangeDesc.Replicas() + votersAndNonVoters := append([]roachpb.ReplicaDescriptor(nil), + replicas.VoterAndNonVoterDescriptors()...) + var learnerReplicaStoreIDs []int + for _, rd := range replicas.LearnerDescriptors() { + learnerReplicaStoreIDs = append(learnerReplicaStoreIDs, int(rd.StoreID)) + } + sort.Slice(votersAndNonVoters, func(i, j int) bool { + return votersAndNonVoters[i].StoreID < votersAndNonVoters[j].StoreID + }) + sort.Ints(learnerReplicaStoreIDs) + votersAndNonVotersArr := tree.NewDArray(types.Int) + for _, replica := range votersAndNonVoters { + if err := votersAndNonVotersArr.Append(tree.NewDInt(tree.DInt(replica.StoreID))); err != nil { + return nil, err + } + } + votersArr := tree.NewDArray(types.Int) + for _, replica := range replicas.VoterDescriptors() { + if err := votersArr.Append(tree.NewDInt(tree.DInt(replica.StoreID))); err != nil { + return nil, err + } + } + nonVotersArr := tree.NewDArray(types.Int) + for _, replica := range replicas.NonVoterDescriptors() { + if err := nonVotersArr.Append(tree.NewDInt(tree.DInt(replica.StoreID))); err != nil { + return nil, err + } + } + learnersArr := tree.NewDArray(types.Int) + for _, replica := range learnerReplicaStoreIDs { + if err := learnersArr.Append(tree.NewDInt(tree.DInt(replica))); err != nil { + return nil, err + } + } + + replicaLocalityArr := tree.NewDArray(types.String) + for _, replica := range votersAndNonVoters { + // The table should still be rendered even if node locality is unavailable, + // so use NULL if nodeDesc is not found. + // See https://github.com/cockroachdb/cockroach/issues/92915. + replicaLocalityDatum := tree.DNull + nodeDesc, err := p.ExecCfg().NodeDescs.GetNodeDescriptor(replica.NodeID) + if err != nil { + if !errorutil.IsDescriptorNotFoundError(err) { + return nil, err + } + } else { + replicaLocalityDatum = tree.NewDString(nodeDesc.Locality.String()) + } + if err := replicaLocalityArr.Append(replicaLocalityDatum); err != nil { + return nil, err + } + } + + tableID, dbName, schemaName, tableName, indexName := lookupNamesByKey( + p, rangeDesc.StartKey.AsRawKey(), dbNames, tableNames, schemaNames, + indexNames, schemaParents, parents, + ) + + splitEnforcedUntil := tree.DNull + if !rangeDesc.StickyBit.IsEmpty() { + splitEnforcedUntil = eval.TimestampToInexactDTimestamp(rangeDesc.StickyBit) + } + + return tree.Datums{ + tree.NewDInt(tree.DInt(rangeDesc.RangeID)), + tree.NewDBytes(tree.DBytes(rangeDesc.StartKey)), + tree.NewDString(keys.PrettyPrint(nil /* valDirs */, rangeDesc.StartKey.AsRawKey())), + tree.NewDBytes(tree.DBytes(rangeDesc.EndKey)), + tree.NewDString(keys.PrettyPrint(nil /* valDirs */, rangeDesc.EndKey.AsRawKey())), + tree.NewDInt(tree.DInt(tableID)), + tree.NewDString(dbName), + tree.NewDString(schemaName), + tree.NewDString(tableName), + tree.NewDString(indexName), + votersAndNonVotersArr, + replicaLocalityArr, + votersArr, + nonVotersArr, + learnersArr, + splitEnforcedUntil, + }, nil + }, nil, nil + }, +} diff --git a/pkg/sql/delegate/BUILD.bazel b/pkg/sql/delegate/BUILD.bazel index c88c5aaa2f64..3a61a3f4a804 100644 --- a/pkg/sql/delegate/BUILD.bazel +++ b/pkg/sql/delegate/BUILD.bazel @@ -21,6 +21,7 @@ go_library( "show_queries.go", "show_range_for_row.go", "show_ranges.go", + "show_ranges_deprecated.go", "show_regions.go", "show_role_grants.go", "show_roles.go", @@ -47,6 +48,7 @@ go_library( "//pkg/sql/catalog/catalogkeys", "//pkg/sql/catalog/colinfo", "//pkg/sql/decodeusername", + "//pkg/sql/deprecatedshowranges", "//pkg/sql/lexbase", "//pkg/sql/oidext", "//pkg/sql/opt/cat", diff --git a/pkg/sql/delegate/delegate.go b/pkg/sql/delegate/delegate.go index 646318eb70e9..6edd7c0aa1f0 100644 --- a/pkg/sql/delegate/delegate.go +++ b/pkg/sql/delegate/delegate.go @@ -13,6 +13,7 @@ package delegate import ( "context" + "github.com/cockroachdb/cockroach/pkg/sql/deprecatedshowranges" "github.com/cockroachdb/cockroach/pkg/sql/opt/cat" "github.com/cockroachdb/cockroach/pkg/sql/parser" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" @@ -98,6 +99,13 @@ func TryDelegate( return d.delegateShowQueries(t) case *tree.ShowRanges: + // Remove in v23.2. + // + // This chooses a different implementation of the SHOW statement + // depending on a run-time config setting. + if deprecatedshowranges.EnableDeprecatedBehavior(ctx, evalCtx.Settings, evalCtx.ClientNoticeSender) { + return d.delegateShowRangesDEPRECATED(t) + } return d.delegateShowRanges(t) case *tree.ShowRangeForRow: diff --git a/pkg/sql/delegate/show_ranges_deprecated.go b/pkg/sql/delegate/show_ranges_deprecated.go new file mode 100644 index 000000000000..a18c9e762be9 --- /dev/null +++ b/pkg/sql/delegate/show_ranges_deprecated.go @@ -0,0 +1,143 @@ +// Copyright 2019 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package delegate + +import ( + "encoding/hex" + "fmt" + + "github.com/cockroachdb/cockroach/pkg/sql/lexbase" + "github.com/cockroachdb/cockroach/pkg/sql/opt/cat" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + "github.com/cockroachdb/cockroach/pkg/sql/privilege" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" + "github.com/cockroachdb/errors" +) + +func checkPrivilegesForShowRangesDEPRECATED(d *delegator, table cat.Table) error { + // Basic requirement is SELECT priviliges + if err := d.catalog.CheckPrivilege(d.ctx, table, privilege.SELECT); err != nil { + return err + } + hasAdmin, err := d.catalog.HasAdminRole(d.ctx) + if err != nil { + return err + } + // User needs to either have admin access or have the correct ZONECONFIG privilege + if hasAdmin { + return nil + } + if err := d.catalog.CheckPrivilege(d.ctx, table, privilege.ZONECONFIG); err != nil { + return pgerror.Wrapf(err, pgcode.InsufficientPrivilege, "only users with the ZONECONFIG privilege or the admin role can use SHOW RANGES on %s", table.Name()) + } + return nil +} + +// delegateShowRangesDEPRECATED implements the SHOW RANGES statement +// using the semantics prior to the introduction of range coalescing. +// +// SHOW RANGES FROM TABLE t +// SHOW RANGES FROM INDEX t@idx +// SHOW RANGES FROM DATABASE db +// +// These statements show the ranges corresponding to the given table or index, +// along with the list of replicas and the lease holder. +func (d *delegator) delegateShowRangesDEPRECATED(n *tree.ShowRanges) (tree.Statement, error) { + switch n.Source { + case tree.ShowRangesDatabase, tree.ShowRangesTable, tree.ShowRangesIndex: + // These are supported by this pre-v23.1 implementation. + default: + return nil, errors.New("the deprecated syntax of SHOW RANGES only supports FROM TABLE, FROM INDEX or FROM DATABASE") + } + + sqltelemetry.IncrementShowCounter(sqltelemetry.Ranges) + if n.DatabaseName != "" { + const dbQuery = ` + SELECT + table_name, + CASE + WHEN crdb_internal.pretty_key(r.start_key, 2) = '' THEN NULL + ELSE crdb_internal.pretty_key(r.start_key, 2) + END AS start_key, + CASE + WHEN crdb_internal.pretty_key(r.end_key, 2) = '' THEN NULL + ELSE crdb_internal.pretty_key(r.end_key, 2) + END AS end_key, + range_id, + range_size / 1000000 as range_size_mb, + lease_holder, + replica_localities[array_position(replicas, lease_holder)] as lease_holder_locality, + replicas, + replica_localities, + voting_replicas, + non_voting_replicas + FROM %[1]s.crdb_internal.ranges AS r + WHERE database_name=%[2]s + ORDER BY table_name, r.start_key + ` + // Note: n.DatabaseName.String() != string(n.DatabaseName) + return d.parse(fmt.Sprintf(dbQuery, n.DatabaseName.String(), lexbase.EscapeSQLString(string(n.DatabaseName)))) + } + + // Remember the original syntax: Resolve below modifies the TableOrIndex struct in-place. + noIndexSpecified := n.TableOrIndex.Index == "" + + idx, resName, err := cat.ResolveTableIndex( + d.ctx, d.catalog, cat.Flags{AvoidDescriptorCaches: true}, &n.TableOrIndex, + ) + if err != nil { + return nil, err + } + + if err := checkPrivilegesForShowRangesDEPRECATED(d, idx.Table()); err != nil { + return nil, err + } + + if idx.Table().IsVirtualTable() { + return nil, errors.New("SHOW RANGES may not be called on a virtual table") + } + + var startKey, endKey string + if noIndexSpecified { + // All indexes. + tableID := idx.Table().ID() + prefix := d.evalCtx.Codec.TablePrefix(uint32(tableID)) + startKey = hex.EncodeToString(prefix) + endKey = hex.EncodeToString(prefix.PrefixEnd()) + } else { + // Just one index. + span := idx.Span() + startKey = hex.EncodeToString(span.Key) + endKey = hex.EncodeToString(span.EndKey) + } + + return d.parse(fmt.Sprintf(` +SELECT + CASE WHEN r.start_key <= x'%[1]s' THEN NULL ELSE crdb_internal.pretty_key(r.start_key, 2) END AS start_key, + CASE WHEN r.end_key >= x'%[2]s' THEN NULL ELSE crdb_internal.pretty_key(r.end_key, 2) END AS end_key, + index_name, + range_id, + range_size / 1000000 as range_size_mb, + lease_holder, + replica_localities[array_position(replicas, lease_holder)] as lease_holder_locality, + replicas, + replica_localities, + voting_replicas, + non_voting_replicas +FROM %[3]s.crdb_internal.ranges AS r +WHERE (r.start_key < x'%[2]s') + AND (r.end_key > x'%[1]s') ORDER BY index_name, r.start_key +`, + startKey, endKey, resName.CatalogName.String(), // note: CatalogName.String() != Catalog() + )) +} diff --git a/pkg/sql/deprecatedshowranges/BUILD.bazel b/pkg/sql/deprecatedshowranges/BUILD.bazel new file mode 100644 index 000000000000..817b3b2d541d --- /dev/null +++ b/pkg/sql/deprecatedshowranges/BUILD.bazel @@ -0,0 +1,21 @@ +load("//build/bazelutil/unused_checker:unused.bzl", "get_x_data") +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "deprecatedshowranges", + srcs = ["condition.go"], + importpath = "github.com/cockroachdb/cockroach/pkg/sql/deprecatedshowranges", + visibility = ["//visibility:public"], + deps = [ + "//pkg/settings", + "//pkg/settings/cluster", + "//pkg/spanconfig/spanconfigstore", + "//pkg/sql/pgwire/pgnotice", + "//pkg/sql/sem/eval", + "//pkg/util/envutil", + "//pkg/util/log", + "@com_github_cockroachdb_errors//:errors", + ], +) + +get_x_data(name = "get_x_data") diff --git a/pkg/sql/deprecatedshowranges/condition.go b/pkg/sql/deprecatedshowranges/condition.go new file mode 100644 index 000000000000..d9c726aeda1d --- /dev/null +++ b/pkg/sql/deprecatedshowranges/condition.go @@ -0,0 +1,122 @@ +// Copyright 2023 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +// Package deprecatedshowranges exists to smoothen the transition +// between the pre-v23.1 semantics of SHOW RANGES and +// crdb_internal.ranges{_no_leases}, and the new semantics introduced +// in v23.1. +// +// The pre-v23.1 semantics are deprecated as of v23.1. At the end of +// the deprecation cycle (hopefully for v23.2) we expect to delete +// this package entirely and all the other code in the SQL layer that +// hangs off the EnableDeprecatedBehavior() conditional defined below. +// +// The mechanism to control the behavior is as follows: +// +// - In any case, an operator can override the behavior using an env +// var, "COCKROACH_FORCE_DEPRECATED_SHOW_RANGE_BEHAVIOR". +// If set (to a boolean), the value of the env var is used in +// all cases. +// We use this env var to force the _new_ behavior through out +// test suite, regardless of the other conditions. This allows +// us to avoid maintaining two sets of outputs in tests. +// +// - If the env var is not set, the cluster setting +// `sql.show_ranges_deprecated_behavior.enabled` is used. It +// defaults to true in v23.1 (the "smoothening" part). +// +// - If the deprecated behavior is chosen by any of the above +// mechanisms, and _the range coalescing cluster setting_ is set +// to true, a loud warning will be reported in various places. +// This will nudge users who wish to opt into range coalescing +// to adapt their use of the range inspection accordingly. +package deprecatedshowranges + +import ( + "context" + "fmt" + "strconv" + + "github.com/cockroachdb/cockroach/pkg/settings" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigstore" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice" + "github.com/cockroachdb/cockroach/pkg/sql/sem/eval" + "github.com/cockroachdb/cockroach/pkg/util/envutil" + "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/errors" +) + +// EnableDeprecatedBehavior chooses whether to use the deprecated +// interface and output for crdb_internal.ranges{_no_leases} and SHOW +// RANGES. See the package-level doc above for details. +func EnableDeprecatedBehavior( + ctx context.Context, st *cluster.Settings, ns eval.ClientNoticeSender, +) bool { + useDeprecatedBehavior := false + if fb.behaviorIsForcedByEnvVar { + useDeprecatedBehavior = fb.useDeprecatedBehavior + } else { + useDeprecatedBehavior = ShowRangesDeprecatedBehaviorSetting.Get(&st.SV) + } + + if useDeprecatedBehavior { + var err pgnotice.Notice + if spanconfigstore.StorageCoalesceAdjacentSetting.Get(&st.SV) { + err = pgnotice.Newf("deprecated use of SHOW RANGES or crdb_internal.ranges{,_no_leases} " + + "in combination with range coalescing - expect invalid results") + // Tell the operator about it in logs. + log.Warningf(ctx, "%v", err) + } else { + err = pgnotice.Newf("attention! the pre-23.1 behavior of SHOW RANGES and crdb_internal.ranges{,_no_leases} is deprecated!") + } + err = errors.WithHint( + err, + "Consider enabling the new functionality by setting "+ + "'sql.show_ranges_deprecated_behavior.enabled' to 'false'.\n"+ + "The new SHOW RANGES statement has more options. "+ + "Refer to the online documentation or execute 'SHOW RANGES ??' for details.") + if ns != nil { + // Tell the SQL client about it via a NOTICE message. + ns.BufferClientNotice(ctx, err) + } + } + + return useDeprecatedBehavior +} + +// ShowRangesDeprecatedBehaviorSetting is the setting that controls the behavior. Exported for use in tests. +var ShowRangesDeprecatedBehaviorSetting = settings.RegisterBoolSetting( + settings.TenantWritable, + `sql.show_ranges_deprecated_behavior.enabled`, + "if set, SHOW RANGES and crdb_internal.ranges{_no_leases} behave with deprecated pre-v23.1 semantics."+ + " NB: the new SHOW RANGES interface has richer WITH options "+ + "than pre-v23.1 SHOW RANGES.", + true, +).WithPublic() + +const envVarName = "COCKROACH_FORCE_DEPRECATED_SHOW_RANGE_BEHAVIOR" + +type forcedBehavior struct { + behaviorIsForcedByEnvVar bool + useDeprecatedBehavior bool +} + +var fb = func() forcedBehavior { + selection, hasEnvVar := envutil.EnvString(envVarName, 1) + if !hasEnvVar { + return forcedBehavior{behaviorIsForcedByEnvVar: false} + } + v, err := strconv.ParseBool(selection) + if err != nil { + panic(fmt.Sprintf("error parsing %s: %s", envVarName, err)) + } + return forcedBehavior{behaviorIsForcedByEnvVar: true, useDeprecatedBehavior: v} +}() diff --git a/pkg/sql/error_hints.go b/pkg/sql/error_hints.go index 1306fd165c4e..860e435432d7 100644 --- a/pkg/sql/error_hints.go +++ b/pkg/sql/error_hints.go @@ -11,16 +11,22 @@ package sql import ( + "context" "strings" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/sql/deprecatedshowranges" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + "github.com/cockroachdb/cockroach/pkg/sql/sem/eval" "github.com/cockroachdb/errors" ) // addPlanningErrorHints is responsible for enhancing the provided // error object with hints that inform the user about their options. -func addPlanningErrorHints(err error, stmt *Statement) error { +func addPlanningErrorHints( + ctx context.Context, err error, stmt *Statement, st *cluster.Settings, ns eval.ClientNoticeSender, +) error { pgCode := pgerror.GetPGCode(err) switch pgCode { case pgcode.UndefinedColumn: @@ -29,7 +35,7 @@ func addPlanningErrorHints(err error, stmt *Statement) error { // Changes introduced in v23.1. extraRangeDoc := false switch { - case strings.Contains(resolvedSQL, "SHOW RANGES"): + case strings.Contains(resolvedSQL, "SHOW RANGES") && !deprecatedshowranges.EnableDeprecatedBehavior(ctx, st, ns): errS := err.Error() // The following columns are not available when using SHOW @@ -40,7 +46,8 @@ func addPlanningErrorHints(err error, stmt *Statement) error { extraRangeDoc = true } - case strings.Contains(resolvedSQL, "crdb_internal.ranges" /* also matches ranges_no_leases */): + case strings.Contains(resolvedSQL, "crdb_internal.ranges" /* also matches ranges_no_leases */) && + !deprecatedshowranges.EnableDeprecatedBehavior(ctx, st, ns): errS := err.Error() // The following columns are not available when using SHOW diff --git a/pkg/sql/event_log.go b/pkg/sql/event_log.go index c08a72aa9fc0..9e970f519982 100644 --- a/pkg/sql/event_log.go +++ b/pkg/sql/event_log.go @@ -192,7 +192,7 @@ var defaultRedactionOptions = redactionOptions{ func (p *planner) getCommonSQLEventDetails(opt redactionOptions) eventpb.CommonSQLEventDetails { redactableStmt := formatStmtKeyAsRedactableString( p.extendedEvalCtx.VirtualSchemas, p.stmt.AST, - p.extendedEvalCtx.Context.Annotations, opt.toFlags(), + p.extendedEvalCtx.Context.Annotations, opt.toFlags(), p, ) commonSQLEventDetails := eventpb.CommonSQLEventDetails{ Statement: redactableStmt, diff --git a/pkg/sql/exec_factory_util.go b/pkg/sql/exec_factory_util.go index 84b08a42d4d0..79fc2151cb56 100644 --- a/pkg/sql/exec_factory_util.go +++ b/pkg/sql/exec_factory_util.go @@ -230,7 +230,7 @@ func constructVirtualScan( delayedNodeCallback func(*delayedNode) (exec.Node, error), ) (exec.Node, error) { tn := &table.(*optVirtualTable).name - virtual, err := p.getVirtualTabler().getVirtualTableEntry(tn) + virtual, err := p.getVirtualTabler().getVirtualTableEntry(tn, p) if err != nil { return nil, err } diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go index 9eb59cba9be8..fe4fd1cbed3d 100644 --- a/pkg/sql/exec_util.go +++ b/pkg/sql/exec_util.go @@ -2232,9 +2232,11 @@ func truncateStatementStringForTelemetry(stmt string) string { // hideNonVirtualTableNameFunc returns a function that can be used with // FmtCtx.SetReformatTableNames. It hides all table names that are not virtual // tables. -func hideNonVirtualTableNameFunc(vt VirtualTabler) func(ctx *tree.FmtCtx, name *tree.TableName) { +func hideNonVirtualTableNameFunc( + vt VirtualTabler, ns eval.ClientNoticeSender, +) func(ctx *tree.FmtCtx, name *tree.TableName) { reformatFn := func(ctx *tree.FmtCtx, tn *tree.TableName) { - virtual, err := vt.getVirtualTableEntry(tn) + virtual, err := vt.getVirtualTableEntry(tn, ns) if err != nil || virtual == nil { // Current table is non-virtual and therefore needs to be scrubbed (for statement stats) or redacted (for logs). @@ -2278,14 +2280,16 @@ func hideNonVirtualTableNameFunc(vt VirtualTabler) func(ctx *tree.FmtCtx, name * return reformatFn } -func anonymizeStmtAndConstants(stmt tree.Statement, vt VirtualTabler) string { +func anonymizeStmtAndConstants( + stmt tree.Statement, vt VirtualTabler, ns eval.ClientNoticeSender, +) string { // Re-format to remove most names. fmtFlags := tree.FmtAnonymize | tree.FmtHideConstants var f *tree.FmtCtx if vt != nil { f = tree.NewFmtCtx( fmtFlags, - tree.FmtReformatTableNames(hideNonVirtualTableNameFunc(vt)), + tree.FmtReformatTableNames(hideNonVirtualTableNameFunc(vt, ns)), ) } else { f = tree.NewFmtCtx(fmtFlags) @@ -2296,8 +2300,10 @@ func anonymizeStmtAndConstants(stmt tree.Statement, vt VirtualTabler) string { // WithAnonymizedStatement attaches the anonymized form of a statement // to an error object. -func WithAnonymizedStatement(err error, stmt tree.Statement, vt VirtualTabler) error { - anonStmtStr := anonymizeStmtAndConstants(stmt, vt) +func WithAnonymizedStatement( + err error, stmt tree.Statement, vt VirtualTabler, ns eval.ClientNoticeSender, +) error { + anonStmtStr := anonymizeStmtAndConstants(stmt, vt, ns) anonStmtStr = truncateStatementStringForTelemetry(anonStmtStr) return errors.WithSafeDetails(err, "while executing: %s", errors.Safe(anonStmtStr)) @@ -3529,7 +3535,7 @@ func quantizeCounts(d *appstatspb.StatementStatistics) { d.FirstAttemptCount = int64((float64(d.FirstAttemptCount) / float64(oldCount)) * float64(newCount)) } -func scrubStmtStatKey(vt VirtualTabler, key string) (string, bool) { +func scrubStmtStatKey(vt VirtualTabler, key string, ns eval.ClientNoticeSender) (string, bool) { // Re-parse the statement to obtain its AST. stmt, err := parser.ParseOne(key) if err != nil { @@ -3539,7 +3545,7 @@ func scrubStmtStatKey(vt VirtualTabler, key string) (string, bool) { // Re-format to remove most names. f := tree.NewFmtCtx( tree.FmtAnonymize, - tree.FmtReformatTableNames(hideNonVirtualTableNameFunc(vt)), + tree.FmtReformatTableNames(hideNonVirtualTableNameFunc(vt, ns)), ) f.FormatNode(stmt.AST) return f.CloseAndGetString(), true @@ -3548,12 +3554,16 @@ func scrubStmtStatKey(vt VirtualTabler, key string) (string, bool) { // formatStmtKeyAsRedactableString given an AST node this function will fully // qualify names using annotations to format it out into a redactable string. func formatStmtKeyAsRedactableString( - vt VirtualTabler, rootAST tree.Statement, ann *tree.Annotations, fs tree.FmtFlags, + vt VirtualTabler, + rootAST tree.Statement, + ann *tree.Annotations, + fs tree.FmtFlags, + ns eval.ClientNoticeSender, ) redact.RedactableString { f := tree.NewFmtCtx( tree.FmtAlwaysQualifyTableNames|tree.FmtMarkRedactionNode|fs, tree.FmtAnnotations(ann), - tree.FmtReformatTableNames(hideNonVirtualTableNameFunc(vt))) + tree.FmtReformatTableNames(hideNonVirtualTableNameFunc(vt, ns))) f.FormatNode(rootAST) formattedRedactableStatementString := f.CloseAndGetString() return redact.RedactableString(formattedRedactableStatementString) diff --git a/pkg/sql/exec_util_test.go b/pkg/sql/exec_util_test.go index 0eee6f78e520..c6e573bcb0ac 100644 --- a/pkg/sql/exec_util_test.go +++ b/pkg/sql/exec_util_test.go @@ -31,7 +31,7 @@ func TestHideNonVirtualTableNameFunc(t *testing.T) { if err != nil { t.Fatal(err) } - tableNameFunc := hideNonVirtualTableNameFunc(vt) + tableNameFunc := hideNonVirtualTableNameFunc(vt, nil) testData := []struct { stmt string diff --git a/pkg/sql/opt_exec_factory.go b/pkg/sql/opt_exec_factory.go index 291a1c3714b8..7820a507daeb 100644 --- a/pkg/sql/opt_exec_factory.go +++ b/pkg/sql/opt_exec_factory.go @@ -777,7 +777,7 @@ func (ef *execFactory) constructVirtualTableLookupJoin( onCond tree.TypedExpr, ) (exec.Node, error) { tn := &table.(*optVirtualTable).name - virtual, err := ef.planner.getVirtualTabler().getVirtualTableEntry(tn) + virtual, err := ef.planner.getVirtualTabler().getVirtualTableEntry(tn, ef.planner) if err != nil { return nil, err } diff --git a/pkg/sql/pg_catalog.go b/pkg/sql/pg_catalog.go index 97edc12bdbcb..eef7eac9185b 100644 --- a/pkg/sql/pg_catalog.go +++ b/pkg/sql/pg_catalog.go @@ -1450,15 +1450,15 @@ https://www.postgresql.org/docs/9.5/catalog-pg-depend.html`, schema: vtable.PGCatalogDepend, populate: func(ctx context.Context, p *planner, dbContext catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error { vt := p.getVirtualTabler() - pgConstraintsDesc, err := vt.getVirtualTableDesc(&pgConstraintsTableName) + pgConstraintsDesc, err := vt.getVirtualTableDesc(&pgConstraintsTableName, p) if err != nil { return errors.New("could not find pg_catalog.pg_constraint") } - pgClassDesc, err := vt.getVirtualTableDesc(&pgClassTableName) + pgClassDesc, err := vt.getVirtualTableDesc(&pgClassTableName, p) if err != nil { return errors.New("could not find pg_catalog.pg_class") } - pgRewriteDesc, err := vt.getVirtualTableDesc(&pgRewriteTableName) + pgRewriteDesc, err := vt.getVirtualTableDesc(&pgRewriteTableName, p) if err != nil { return errors.New("could not find pg_catalog.pg_rewrite") } @@ -2892,19 +2892,19 @@ https://www.postgresql.org/docs/9.6/catalog-pg-shdepend.html`, vt := p.getVirtualTabler() h := makeOidHasher() - pgClassDesc, err := vt.getVirtualTableDesc(&pgClassTableName) + pgClassDesc, err := vt.getVirtualTableDesc(&pgClassTableName, p) if err != nil { return errors.New("could not find pg_catalog.pg_class") } pgClassOid := tableOid(pgClassDesc.GetID()) - pgAuthIDDesc, err := vt.getVirtualTableDesc(&pgAuthIDTableName) + pgAuthIDDesc, err := vt.getVirtualTableDesc(&pgAuthIDTableName, p) if err != nil { return errors.New("could not find pg_catalog.pg_authid") } pgAuthIDOid := tableOid(pgAuthIDDesc.GetID()) - pgDatabaseDesc, err := vt.getVirtualTableDesc(&pgDatabaseTableName) + pgDatabaseDesc, err := vt.getVirtualTableDesc(&pgDatabaseTableName, p) if err != nil { return errors.New("could not find pg_catalog.pg_database") } diff --git a/pkg/sql/schema_change_plan_node.go b/pkg/sql/schema_change_plan_node.go index 66e8e9fcfd65..75b7a76c79cd 100644 --- a/pkg/sql/schema_change_plan_node.go +++ b/pkg/sql/schema_change_plan_node.go @@ -46,7 +46,7 @@ func (p *planner) FormatAstAsRedactableString( ) redact.RedactableString { return formatStmtKeyAsRedactableString(p.getVirtualTabler(), statement, - annotations, tree.FmtSimple) + annotations, tree.FmtSimple, p) } // SchemaChange provides the planNode for the new schema changer. diff --git a/pkg/sql/statement_mark_redaction_test.go b/pkg/sql/statement_mark_redaction_test.go index eea28917b02c..3fd222b23440 100644 --- a/pkg/sql/statement_mark_redaction_test.go +++ b/pkg/sql/statement_mark_redaction_test.go @@ -97,7 +97,7 @@ func TestMarkRedactionStatement(t *testing.T) { f := tree.NewFmtCtx( tree.FmtAlwaysQualifyTableNames|tree.FmtMarkRedactionNode, tree.FmtAnnotations(&ann), - tree.FmtReformatTableNames(hideNonVirtualTableNameFunc(vt))) + tree.FmtReformatTableNames(hideNonVirtualTableNameFunc(vt, nil))) f.FormatNode(stmt.AST) redactedString := f.CloseAndGetString() require.Equal(t, test.expected, redactedString) diff --git a/pkg/sql/virtual_schema.go b/pkg/sql/virtual_schema.go index 6fbe306e6c56..2302ac80ce62 100644 --- a/pkg/sql/virtual_schema.go +++ b/pkg/sql/virtual_schema.go @@ -29,6 +29,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/schemaexpr" "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" "github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc" + "github.com/cockroachdb/cockroach/pkg/sql/deprecatedshowranges" "github.com/cockroachdb/cockroach/pkg/sql/opt/constraint" "github.com/cockroachdb/cockroach/pkg/sql/parser" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" @@ -407,6 +408,10 @@ type VirtualSchemaHolder struct { schemasByID map[descpb.ID]*virtualSchemaEntry defsByID map[descpb.ID]*virtualDefEntry orderedNames []string + + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + st *cluster.Settings } var _ VirtualTabler = (*VirtualSchemaHolder)(nil) @@ -429,6 +434,18 @@ func (vs *VirtualSchemaHolder) GetVirtualObjectByID(id descpb.ID) (catalog.Virtu if !ok { return nil, false } + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + switch { + // We only check the Enable condition after we know we're + // looking at the "interesting" ID because the Enable condition + // has side effects. + case id == catconstants.CrdbInternalRangesViewID && deprecatedshowranges.EnableDeprecatedBehavior(context.TODO(), vs.st, nil): + entry = vs.schemasByID[catconstants.CrdbInternalID].deprecatedRangesDef + case id == catconstants.CrdbInternalRangesNoLeasesTableID && deprecatedshowranges.EnableDeprecatedBehavior(context.TODO(), vs.st, nil): + entry = vs.schemasByID[catconstants.CrdbInternalID].deprecatedRangesNoLeasesDef + } + return entry, true } @@ -439,6 +456,10 @@ func (vs *VirtualSchemaHolder) Visit(fn func(desc catalog.Descriptor, comment st return iterutil.Map(err) } for _, def := range sc.defs { + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + def = sc.compatSubstDef(def, nil /* ClientNoticeSender */) + if err := fn(def.desc, def.comment); err != nil { return iterutil.Map(err) } @@ -455,6 +476,12 @@ type virtualSchemaEntry struct { orderedDefNames []string undefinedTables map[string]struct{} containsTypes bool + + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + st *cluster.Settings + deprecatedRangesDef *virtualDefEntry + deprecatedRangesNoLeasesDef *virtualDefEntry } func (v *virtualSchemaEntry) Desc() catalog.SchemaDescriptor { @@ -467,7 +494,12 @@ func (v *virtualSchemaEntry) NumTables() int { func (v *virtualSchemaEntry) VisitTables(f func(object catalog.VirtualObject)) { for _, name := range v.orderedDefNames { - f(v.defs[name]) + def := v.defs[name] + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + def = v.compatSubstDef(def, nil /* ClientNoticeSender */) + + f(def) } } @@ -504,6 +536,10 @@ func (v *virtualSchemaEntry) GetObjectByName( fallthrough case tree.TableObject: if def, ok := v.defs[name]; ok { + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + def = v.compatSubstDef(def, nil /* ClientNoticeSender */) + return def, nil } if _, ok := v.undefinedTables[name]; ok { @@ -880,6 +916,10 @@ func NewVirtualSchemaHolder( schemasByID: make(map[descpb.ID]*virtualSchemaEntry, len(virtualSchemas)), orderedNames: make([]string, len(virtualSchemas)), defsByID: make(map[descpb.ID]*virtualDefEntry, math.MaxUint32-catconstants.MinVirtualID), + + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + st: st, } order := 0 @@ -895,24 +935,24 @@ func NewVirtualSchemaHolder( defs := make(map[string]*virtualDefEntry, len(schema.tableDefs)) orderedDefNames := make([]string, 0, len(schema.tableDefs)) - for id, def := range schema.tableDefs { + doTheWork := func(id descpb.ID, def virtualSchemaDef) (tb descpb.TableDescriptor, de *virtualDefEntry, err error) { tableDesc, err := def.initVirtualTableDesc(ctx, st, scDesc, id) if err != nil { - return nil, errors.NewAssertionErrorWithWrappedErrf(err, + return tb, nil, errors.NewAssertionErrorWithWrappedErrf(err, "failed to initialize %s", errors.Safe(def.getSchema())) } if schema.tableValidator != nil { if err := schema.tableValidator(&tableDesc); err != nil { - return nil, errors.NewAssertionErrorWithWrappedErrf(err, "programmer error") + return tb, nil, errors.NewAssertionErrorWithWrappedErrf(err, "programmer error") } } td := tabledesc.NewBuilder(&tableDesc).BuildImmutableTable() version := st.Version.ActiveVersionOrEmpty(ctx) dvmp := catsessiondata.NewDescriptorSessionDataProvider(nil /* sd */) if err := descs.ValidateSelf(td, version, dvmp); err != nil { - return nil, errors.NewAssertionErrorWithWrappedErrf(err, + return tb, nil, errors.NewAssertionErrorWithWrappedErrf(err, "failed to validate virtual table %s: programmer error", errors.Safe(td.GetName())) } @@ -923,6 +963,14 @@ func NewVirtualSchemaHolder( comment: def.getComment(), unimplemented: def.isUnimplemented(), } + return tableDesc, entry, nil + } + + for id, def := range schema.tableDefs { + tableDesc, entry, err := doTheWork(id, def) + if err != nil { + return nil, err + } defs[tableDesc.Name] = entry vs.defsByID[tableDesc.ID] = entry orderedDefNames = append(orderedDefNames, tableDesc.Name) @@ -930,12 +978,27 @@ func NewVirtualSchemaHolder( sort.Strings(orderedDefNames) + _, extra1, err := doTheWork(catconstants.CrdbInternalRangesViewID, crdbInternalRangesViewDEPRECATED) + if err != nil { + return nil, err + } + _, extra2, err := doTheWork(catconstants.CrdbInternalRangesNoLeasesTableID, crdbInternalRangesNoLeasesTableDEPRECATED) + if err != nil { + return nil, err + } + vse := &virtualSchemaEntry{ desc: scDesc, defs: defs, orderedDefNames: orderedDefNames, undefinedTables: schema.undefinedTables, containsTypes: schema.containsTypes, + + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + st: st, + deprecatedRangesDef: extra1, + deprecatedRangesNoLeasesDef: extra2, } vs.schemasByName[scDesc.GetName()] = vse vs.schemasByID[scDesc.GetID()] = vse @@ -972,15 +1035,39 @@ func (vs *VirtualSchemaHolder) getVirtualSchemaEntry(name string) (*virtualSchem return e, ok } +// Needed for backward-compat on crdb_internal.ranges{_no_leases}. +// Remove in v23.2. +func (v *virtualSchemaEntry) compatSubstDef( + t *virtualDefEntry, ns eval.ClientNoticeSender, +) *virtualDefEntry { + switch { + // We only check the Enable condition after we know we're + // looking at the "interesting" ID because the Enable condition + // has side effects. + case t.desc.GetID() == catconstants.CrdbInternalRangesViewID && deprecatedshowranges.EnableDeprecatedBehavior(context.TODO(), v.st, ns): + t = v.deprecatedRangesDef + + case t.desc.GetID() == catconstants.CrdbInternalRangesNoLeasesTableID && deprecatedshowranges.EnableDeprecatedBehavior(context.TODO(), v.st, ns): + t = v.deprecatedRangesNoLeasesDef + } + return t +} + // getVirtualTableEntry checks if the provided name matches a virtual database/table // pair. The function will return the table's virtual table entry if the name matches // a specific table. It will return an error if the name references a virtual database // but the table is non-existent. // getVirtualTableEntry is part of the VirtualTabler interface. -func (vs *VirtualSchemaHolder) getVirtualTableEntry(tn *tree.TableName) (*virtualDefEntry, error) { +func (vs *VirtualSchemaHolder) getVirtualTableEntry( + tn *tree.TableName, ns eval.ClientNoticeSender, +) (*virtualDefEntry, error) { if db, ok := vs.getVirtualSchemaEntry(tn.Schema()); ok { tableName := tn.Table() if t, ok := db.defs[tableName]; ok { + // Needed for backward-compat on crdb_internal.ranges{_no_leases}. + // Remove in v23.2. + t = db.compatSubstDef(t, ns) + sqltelemetry.IncrementGetVirtualTableEntry(tn.Schema(), tableName) return t, nil } @@ -1000,8 +1087,8 @@ func (vs *VirtualSchemaHolder) getVirtualTableEntry(tn *tree.TableName) (*virtua // VirtualTabler is used to fetch descriptors for virtual tables and databases. type VirtualTabler interface { - getVirtualTableDesc(tn *tree.TableName) (catalog.TableDescriptor, error) - getVirtualTableEntry(tn *tree.TableName) (*virtualDefEntry, error) + getVirtualTableDesc(tn *tree.TableName, ns eval.ClientNoticeSender) (catalog.TableDescriptor, error) + getVirtualTableEntry(tn *tree.TableName, ns eval.ClientNoticeSender) (*virtualDefEntry, error) getSchemas() map[string]*virtualSchemaEntry getSchemaNames() []string } @@ -1010,9 +1097,9 @@ type VirtualTabler interface { // pair, and returns its descriptor if it does. // getVirtualTableDesc is part of the VirtualTabler interface. func (vs *VirtualSchemaHolder) getVirtualTableDesc( - tn *tree.TableName, + tn *tree.TableName, ns eval.ClientNoticeSender, ) (catalog.TableDescriptor, error) { - t, err := vs.getVirtualTableEntry(tn) + t, err := vs.getVirtualTableEntry(tn, ns) if err != nil || t == nil { return nil, err }