sql: backward-compatibility for SHOW RANGES / crdb_internal.ranges #98979

knz · 2023-03-19T13:15:12Z

Requested by @mwang1026 and @nvanbenschoten .

Fixes #97861.
Fixes #99073.
Needed for #98820.
Epic: CRDB-24928

See the individual commits for details; exceprt from the 2nd commit:

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 by default in
cockroach demo, i.e. the tests continue to exercise the new
behavior.

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 SHWO RANGES can be enabled by changing the cluster
setting sql.show_ranges_deprecated_behavior.enabled to false. It
will become default in v23.2.

cockroach-teamcity · 2023-03-19T13:15:20Z

This change is

cucaroach

fqazi

Nothing major just minor nits

Reviewed 4 of 4 files at r1, 25 of 25 files at r2, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @irfansharif and @knz)

pkg/sql/crdb_internal_ranges_deprecated.go line 34 at r2 (raw file):

// version of `crdb_internal.ranges`. See the package-level doc
// of package `deprecatedshowranges` for details.
var crdbInternalRangesViewDEPRECATED = virtualSchemaView{

Maybe just add a version number instead and have a TODO for removal

pkg/sql/virtual_schema.go line 445 at r2 (raw file):

	case id == catconstants.CrdbInternalRangesViewID && deprecatedshowranges.EnableDeprecatedBehavior(context.TODO(), vs.st, nil):
		entry = vs.schemasByID[catconstants.CrdbInternalID].extraRangesDef
	case id == catconstants.CrdbInternalRangesNoLeasesTableID && deprecatedshowranges.EnableDeprecatedBehavior(context.TODO(), vs.st, nil):

Can we pipe in a context too?

pkg/sql/virtual_schema.go line 1000 at r2 (raw file):

			// Remove in v23.2.
			st:                     st,
			extraRangesDef:         extra1,

Instead of extra just label them as deprecated or with the version

knz

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @fqazi and @irfansharif)

pkg/sql/crdb_internal_ranges_deprecated.go line 34 at r2 (raw file):

Previously, fqazi (Faizan Qazi) wrote…

Maybe just add a version number instead and have a TODO for removal

Done.

pkg/sql/virtual_schema.go line 445 at r2 (raw file):

Previously, fqazi (Faizan Qazi) wrote…

Can we pipe in a context too?

I tried - but then I need to change the interface in catalog which then requires me to change (*byIDLookupContext).lookupVirtual in catalog/desc, which then requires me to change all the other lookupFuncs. That's too much change for this PR.

pkg/sql/virtual_schema.go line 1000 at r2 (raw file):

Previously, fqazi (Faizan Qazi) wrote…

Instead of extra just label them as deprecated or with the version

Done.

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: ``` [email protected]: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. ``` ``` [email protected]: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

…eases}` 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.

knz · 2023-03-26T19:04:16Z

TYFRs!

bors r=cucaroach,fqazi

craig · 2023-03-26T19:34:47Z

Build succeeded:

Bazel Essential CI (Cockroach)

mikeCRL · 2023-05-12T15:42:48Z

@knz Is there a reason to keep this in Backward-Incompatible Changes rather than Deprecations? It looks like we maintain the behavior of v22.2 by default in v23.1, though it is deprecated in v23.1 and will change by default in v23.2.

knz · 2023-05-12T15:45:11Z

Is there a reason to keep this in Backward-Incompatible Changes rather than Deprecations?

I chose that category to increase the chance the doc writer who sees it will go back to the earlier release note in the release cycle that was marking the output as deprecated, and revise it.

Customers are going to look at the combined list of deprecation notices across the entire 23.1 release and we need to build a consistent messaging, by editing the earlier release note.

mikeCRL · 2023-05-12T15:52:47Z

@knz I see. Thanks. Indeed I'd aggregated all prior Backward-Incompatible Changes and that captured all the SHOW RANGES changes. Other than bumping up the last SHOW RANGES bullet so that it falls directly after the others, would you recommend any other changes to what we have in the draft for that section? (Preview / PR)

knz · 2023-05-12T16:12:41Z

i added some comments to that PR thanks

knz requested review from irfansharif and cucaroach March 19, 2023 13:15

knz changed the title ~~sql: introduce planning error hints~~ sql: introduce planning error hints and hint at the changes in SHOW RANGES / crdb_internal.ranges Mar 19, 2023

knz requested a review from ecwall March 20, 2023 14:06

knz added the A-multitenancy Related to multi-tenancy label Mar 20, 2023

cucaroach approved these changes Mar 21, 2023

View reviewed changes

knz force-pushed the 20230319-ranges-notice branch from f0e22c3 to abca78d Compare March 22, 2023 23:17

knz requested review from a team as code owners March 22, 2023 23:17

knz requested a review from a team March 22, 2023 23:17

knz requested review from a team as code owners March 22, 2023 23:17

knz requested a review from a team March 22, 2023 23:17

knz requested a review from a team as a code owner March 22, 2023 23:17

knz requested review from herkolategan and renatolabs and removed request for a team March 22, 2023 23:17

knz changed the title ~~sql: introduce planning error hints and hint at the changes in SHOW RANGES / crdb_internal.ranges~~ sql: backward-compatibility for SHOW RANGES / crdb_internal.ranges Mar 22, 2023

knz requested review from fqazi and removed request for ecwall, a team, herkolategan and renatolabs March 22, 2023 23:20

knz force-pushed the 20230319-ranges-notice branch from abca78d to c575203 Compare March 22, 2023 23:22

knz added the backport-23.1.x Flags PRs that need to be backported to 23.1 label Mar 22, 2023

fqazi approved these changes Mar 22, 2023

View reviewed changes

knz force-pushed the 20230319-ranges-notice branch 2 times, most recently from d32e975 to 68e8730 Compare March 26, 2023 11:54

knz requested a review from a team as a code owner March 26, 2023 11:54

knz requested a review from a team March 26, 2023 11:54

knz commented Mar 26, 2023

View reviewed changes

knz force-pushed the 20230319-ranges-notice branch from 68e8730 to 3d12987 Compare March 26, 2023 11:59

knz force-pushed the 20230319-ranges-notice branch from 3d12987 to 4517873 Compare March 26, 2023 13:46

knz force-pushed the 20230319-ranges-notice branch from 4517873 to 87afa37 Compare March 26, 2023 19:04

craig bot merged commit 2bd2c80 into cockroachdb:master Mar 26, 2023

blathers-crl bot mentioned this pull request Mar 26, 2023

release-23.1: sql: backward-compatibility for SHOW RANGES / crdb_internal.ranges #99618

Merged

knz deleted the 20230319-ranges-notice branch March 26, 2023 19:39

cockroach-teamcity mentioned this pull request Mar 27, 2023

PR #98979 - sql: backward-compat for SHOW RANGES / `crdb_internal.ranges{,_no_l… cockroachdb/docs#16611

Closed

jaylim-crl mentioned this pull request May 5, 2023

sql: add a placeholder cluster setting for sql.show_ranges_deprecated_behavior.enabled in v22.2 #102818

Closed

mikeCRL mentioned this pull request May 12, 2023

23.1 GA Release Notes cockroachdb/docs#16951

Merged

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sql: backward-compatibility for SHOW RANGES / crdb_internal.ranges #98979

sql: backward-compatibility for SHOW RANGES / crdb_internal.ranges #98979

knz commented Mar 19, 2023 •

edited

Loading

cockroach-teamcity commented Mar 19, 2023

cucaroach left a comment •

edited by cockroach-dev-inf

Loading

fqazi left a comment

knz left a comment

knz commented Mar 26, 2023

craig bot commented Mar 26, 2023

mikeCRL commented May 12, 2023

knz commented May 12, 2023

mikeCRL commented May 12, 2023 •

edited

Loading

knz commented May 12, 2023

sql: backward-compatibility for SHOW RANGES / crdb_internal.ranges #98979

sql: backward-compatibility for SHOW RANGES / crdb_internal.ranges #98979

Conversation

knz commented Mar 19, 2023 • edited Loading

cockroach-teamcity commented Mar 19, 2023

cucaroach left a comment • edited by cockroach-dev-inf Loading

Choose a reason for hiding this comment

fqazi left a comment

Choose a reason for hiding this comment

knz left a comment

Choose a reason for hiding this comment

knz commented Mar 26, 2023

craig bot commented Mar 26, 2023

mikeCRL commented May 12, 2023

knz commented May 12, 2023

mikeCRL commented May 12, 2023 • edited Loading

knz commented May 12, 2023

knz commented Mar 19, 2023 •

edited

Loading

cucaroach left a comment •

edited by cockroach-dev-inf

Loading

mikeCRL commented May 12, 2023 •

edited

Loading