Merge cockroachdb#97819

97819: clusterversion: remove three 22_2 version gates owned by SQL Queries r=yuzefovich a=yuzefovich This commit removes the following version gates: - V22_2TrigramInvertedIndexes - V22_2AlterSystemSQLInstancesAddLocality (as well as the corresponding migration) - V22_2DeleteRequestReturnKey. Fixes: cockroachdb#96757. Release note: None Co-authored-by: Yahor Yuzefovich <[email protected]>
jayshrivastava · Mar 7, 2023 · 43a25a0 · 43a25a0
2 parents c078537 + 33f6af8
commit 43a25a0
Show file tree

Hide file tree

Showing 9 changed files with 3 additions and 238 deletions.
diff --git a/pkg/clusterversion/cockroach_versions.go b/pkg/clusterversion/cockroach_versions.go
@@ -199,9 +199,6 @@ const (
 	// version is guaranteed to reside in a cluster where all nodes support range
 	// keys at the Pebble layer.
 	TODODelete_V22_2EnablePebbleFormatVersionRangeKeys
-	// TODODelete_V22_2TrigramInvertedIndexes enables the creation of trigram inverted indexes
-	// on strings.
-	TODODelete_V22_2TrigramInvertedIndexes
 	// TODODelete_V22_2RemoveGrantPrivilege is the last step to migrate from the GRANT privilege to WITH GRANT OPTION.
 	TODODelete_V22_2RemoveGrantPrivilege
 	// TODODelete_V22_2MVCCRangeTombstones enables the use of MVCC range tombstones.
@@ -220,9 +217,6 @@ const (
 	// TODODelete_V22_2EnablePredicateProjectionChangefeed indicates that changefeeds support
 	// predicates and projections.
 	TODODelete_V22_2EnablePredicateProjectionChangefeed
-	// TODODelete_V22_2AlterSystemSQLInstancesAddLocality adds a locality column to the
-	// system.sql_instances table.
-	TODODelete_V22_2AlterSystemSQLInstancesAddLocality
 	// TODODelete_V22_2SystemExternalConnectionsTable adds system.external_connections table.
 	TODODelete_V22_2SystemExternalConnectionsTable
 	// TODODelete_V22_2RoleIDSequence is the version where the system.role_id_sequence exists.
@@ -241,9 +235,6 @@ const (
 	// TODODelete_V22_2SchemaChangeSupportsCreateFunction adds support of CREATE FUNCTION
 	// statement.
 	TODODelete_V22_2SchemaChangeSupportsCreateFunction
-	// TODODelete_V22_2DeleteRequestReturnKey is the version where the DeleteRequest began
-	// populating the FoundKey value in the response.
-	TODODelete_V22_2DeleteRequestReturnKey
 	// TODODelete_V22_2PebbleFormatPrePebblev1Marked performs a Pebble-level migration and
 	// upgrades the Pebble format major version to FormatPrePebblev1Marked. This
 	// migration occurs at the per-store level and is twofold:
@@ -552,10 +543,6 @@ var rawVersionsSingleton = keyedVersions{
 		Key:     TODODelete_V22_2EnablePebbleFormatVersionRangeKeys,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 10},
 	},
-	{
-		Key:     TODODelete_V22_2TrigramInvertedIndexes,
-		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 12},
-	},
 	{
 		Key:     TODODelete_V22_2RemoveGrantPrivilege,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 14},
@@ -580,10 +567,6 @@ var rawVersionsSingleton = keyedVersions{
 		Key:     TODODelete_V22_2EnablePredicateProjectionChangefeed,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 26},
 	},
-	{
-		Key:     TODODelete_V22_2AlterSystemSQLInstancesAddLocality,
-		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 28},
-	},
 	{
 		Key:     TODODelete_V22_2SystemExternalConnectionsTable,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 30},
@@ -612,10 +595,6 @@ var rawVersionsSingleton = keyedVersions{
 		Key:     TODODelete_V22_2SchemaChangeSupportsCreateFunction,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 44},
 	},
-	{
-		Key:     TODODelete_V22_2DeleteRequestReturnKey,
-		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 46},
-	},
 	{
 		Key:     TODODelete_V22_2PebbleFormatPrePebblev1Marked,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 48},

diff --git a/pkg/sql/create_index.go b/pkg/sql/create_index.go
@@ -393,11 +393,6 @@ func populateInvertedIndexDescriptor(
 		// we're going to inverted index.
 		switch invCol.OpClass {
 		case "gin_trgm_ops", "gist_trgm_ops":
-			if !cs.Version.IsActive(ctx, clusterversion.TODODelete_V22_2TrigramInvertedIndexes) {
-				return pgerror.Newf(pgcode.FeatureNotSupported,
-					"version %v must be finalized to create trigram inverted indexes",
-					clusterversion.ByKey(clusterversion.TODODelete_V22_2TrigramInvertedIndexes))
-			}
 		case "":
 			return errors.WithHint(
 				pgerror.New(pgcode.UndefinedObject, "data type text has no default operator class for access method \"gin\""),

diff --git a/pkg/sql/delete_range.go b/pkg/sql/delete_range.go
@@ -14,7 +14,6 @@ import (
 	"bytes"
 	"context"
 
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/kv"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
@@ -166,9 +165,8 @@ func (d *deleteRangeNode) startExec(params runParams) error {
 func (d *deleteRangeNode) deleteSpans(params runParams, b *kv.Batch, spans roachpb.Spans) {
 	ctx := params.ctx
 	traceKV := params.p.ExtendedEvalContext().Tracing.KVTracingEnabled()
-	canUsePointDelete := params.ExecCfg().Settings.Version.IsActive(ctx, clusterversion.TODODelete_V22_2DeleteRequestReturnKey)
 	for _, span := range spans {
-		if span.EndKey == nil && canUsePointDelete {
+		if span.EndKey == nil {
 			if traceKV {
 				log.VEventf(ctx, 2, "Del %s", span.Key)
 			}

diff --git a/pkg/sql/opt_exec_factory.go b/pkg/sql/opt_exec_factory.go
@@ -19,7 +19,6 @@ import (
 	"net/url"
 	"strings"
 
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/featureflag"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
@@ -1759,15 +1758,9 @@ func (ef *execFactory) ConstructDeleteRange(
 	var sb span.Builder
 	sb.Init(ef.planner.EvalContext(), ef.planner.ExecCfg().Codec, tabDesc, tabDesc.GetPrimaryIndex())
 
-	splitter := span.NoopSplitter()
-	canUsePointDelete := ef.planner.ExecCfg().Settings.Version.IsActive(
-		ef.ctx, clusterversion.TODODelete_V22_2DeleteRequestReturnKey,
+	splitter := span.MakeSplitterForDelete(
+		tabDesc, tabDesc.GetPrimaryIndex(), needed, true, /* forDelete */
 	)
-	if canUsePointDelete {
-		splitter = span.MakeSplitterForDelete(
-			tabDesc, tabDesc.GetPrimaryIndex(), needed, true, /* forDelete */
-		)
-	}
 	spans, err := sb.SpansFromConstraint(indexConstraint, splitter)
 	if err != nil {
 		return nil, err

diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_index.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_index.go
@@ -16,7 +16,6 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/docs"
 	"github.com/cockroachdb/cockroach/pkg/geo/geoindex"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
@@ -346,11 +345,6 @@ func processColNodeType(
 			// we're going to inverted index.
 			switch columnNode.OpClass {
 			case "gin_trgm_ops", "gist_trgm_ops":
-				if !b.EvalCtx().Settings.Version.IsActive(b, clusterversion.TODODelete_V22_2TrigramInvertedIndexes) {
-					panic(pgerror.Newf(pgcode.FeatureNotSupported,
-						"version %v must be finalized to create trigram inverted indexes",
-						clusterversion.ByKey(clusterversion.TODODelete_V22_2TrigramInvertedIndexes)))
-				}
 			case "":
 				panic(errors.WithHint(
 					pgerror.New(pgcode.UndefinedObject, "data type text has no default operator class for access method \"gin\""),

diff --git a/pkg/upgrade/upgrades/BUILD.bazel b/pkg/upgrade/upgrades/BUILD.bazel
@@ -5,7 +5,6 @@ go_library(
     name = "upgrades",
     srcs = [
         "alter_jobs_add_job_type.go",
-        "alter_sql_instances_locality.go",
         "alter_sql_instances_sql_addr.go",
         "alter_table_statistics_partial_predicate_and_id.go",
         "create_index_usage_statement_statistics.go",
@@ -88,7 +87,6 @@ go_test(
     size = "large",
     srcs = [
         "alter_jobs_add_job_type_test.go",
-        "alter_sql_instances_locality_test.go",
         "alter_sql_instances_sql_addr_test.go",
         "alter_table_statistics_partial_predicate_and_id_test.go",
         "builtins_test.go",

diff --git a/pkg/upgrade/upgrades/alter_sql_instances_locality.go b/pkg/upgrade/upgrades/alter_sql_instances_locality.go
diff --git a/pkg/upgrade/upgrades/alter_sql_instances_locality_test.go b/pkg/upgrade/upgrades/alter_sql_instances_locality_test.go
diff --git a/pkg/upgrade/upgrades/upgrades.go b/pkg/upgrade/upgrades/upgrades.go
@@ -111,12 +111,6 @@ var upgrades = []upgradebase.Upgrade{
 		upgrade.NoPrecondition,
 		sampledStmtDiagReqsMigration,
 	),
-	upgrade.NewTenantUpgrade(
-		"add column locality to table system.sql_instances",
-		toCV(clusterversion.TODODelete_V22_2AlterSystemSQLInstancesAddLocality),
-		upgrade.NoPrecondition,
-		alterSystemSQLInstancesAddLocality,
-	),
 	upgrade.NewTenantUpgrade(
 		"add the system.external_connections table",
 		toCV(clusterversion.TODODelete_V22_2SystemExternalConnectionsTable),