sql: adding version gates for changes related to JSON forward indexes

Previously, #99275 and #97298 were responsible for laying the
foundations of a JSON lexicographical order as well as enabling JSON
forward indexes in CRDB. However, these commits did not account for
version gates. In a given cluster, all of its nodes are required to be
at a certain version number for creating JSON forward indexes as well as
for ordering JSON columns.  Thus, this PR aims to enable version gates
for ensuring all nodes in a given cluster meet this requirement.

Epic: CRDB-24501

Release note (sql change): This PR adds version gates which requires all
nodes in a given cluster to have a minimum binary version number which
in turn is required for creating forward indexes on JSON columns and for
ordering JSON columns.

Fixes: #35706

docs/generated/settings/settings-for-tenants.txt

@@ -293,4 +293,4 @@ trace.opentelemetry.collector	string		address of an OpenTelemetry trace collecto
 trace.snapshot.rate	duration	0s	if non-zero, interval at which background trace snapshots are captured	tenant-rw
 trace.span_registry.enabled	boolean	true	if set, ongoing traces can be seen at https://<ui>/#/debug/tracez	tenant-rw
 trace.zipkin.collector	string		the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.	tenant-rw
-version	version	1000023.1-2	set the active cluster version in the format '<major>.<minor>'	tenant-rw
+version	version	1000023.2-4	set the active cluster version in the format '<major>.<minor>'	tenant-rw

docs/generated/settings/settings.html

@@ -246,6 +246,6 @@
 <tr><td><div id="setting-trace-snapshot-rate" class="anchored"><code>trace.snapshot.rate</code></div></td><td>duration</td><td><code>0s</code></td><td>if non-zero, interval at which background trace snapshots are captured</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-trace-span-registry-enabled" class="anchored"><code>trace.span_registry.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>if set, ongoing traces can be seen at https://&lt;ui&gt;/#/debug/tracez</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-trace-zipkin-collector" class="anchored"><code>trace.zipkin.collector</code></div></td><td>string</td><td><code></code></td><td>the address of a Zipkin instance to receive traces, as &lt;host&gt;:&lt;port&gt;. If no port is specified, 9411 will be used.</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
-<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000023.1-2</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
+<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000023.2-4</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 </tbody>
 </table>

pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant

@@ -391,7 +391,7 @@ select crdb_internal.get_vmodule()
 query T
 select regexp_replace(regexp_replace(crdb_internal.node_executable_version()::string, '(-\d+)?$', ''), '10000', '');
 ----
-23.1
+23.2
 
 query ITTT colnames
 select node_id, component, field, regexp_replace(regexp_replace(value, '^\d+$', '<port>'), e':\\d+', ':<port>') as value from crdb_internal.node_runtime_info
@@ -484,7 +484,7 @@ select * from crdb_internal.gossip_alerts
 query T
 select regexp_replace(regexp_replace(crdb_internal.node_executable_version()::string, '(-\d+)?$', ''), '10000', '');
 ----
-23.1
+23.2
 
 user root
 

pkg/cli/testdata/declarative-rules/deprules

@@ -1,6 +1,6 @@
 dep
 ----
-debug declarative-print-rules 1000023.1-2 dep
+debug declarative-print-rules 1000023.2-4 dep
 deprules
 ----
 - name: 'CheckConstraint transitions to ABSENT uphold 2-version invariant: PUBLIC->VALIDATED'

pkg/cli/testdata/declarative-rules/oprules

@@ -1,6 +1,6 @@
 op
 ----
-debug declarative-print-rules 1000023.1-2 op
+debug declarative-print-rules 1000023.2-4 op
 rules
 ----
 []

pkg/clusterversion/cockroach_versions.go

@@ -540,6 +540,10 @@ const (
 	// the process of upgrading from previous supported releases to 23.2.
 	V23_2Start
 
+	// V23_2_JSONForwardIndexes enables forward indexing for JSON columns, along with a
+	// lexicographical ordering for JSON columns, for all clusters with a version >= 23.2
+	V23_2_JSONForwardIndexes
+
 	// *************************************************
 	// Step 1b: Add new version for 23.2 development here.
 	// Do not add new versions to a patch release.
@@ -942,6 +946,10 @@ var rawVersionsSingleton = keyedVersions{
 		Key:     V23_2Start,
 		Version: roachpb.Version{Major: 23, Minor: 1, Internal: 2},
 	},
+	{
+		Key:     V23_2_JSONForwardIndexes,
+		Version: roachpb.Version{Major: 23, Minor: 1, Internal: 4},
+	},
 
 	// *************************************************
 	// Step 2b: Add new version gates for 23.2 development here.

pkg/sql/create_index.go

@@ -189,7 +189,7 @@ func makeIndexDescriptor(
 		return nil, pgerror.Newf(pgcode.DuplicateRelation, "index with name %q already exists", n.Name)
 	}
 
-	if err := checkIndexColumns(tableDesc, columns, n.Storing, n.Inverted); err != nil {
+	if err := checkIndexColumns(tableDesc, columns, n.Storing, n.Inverted, params.ExecCfg().Settings.Version.ActiveVersion(params.ctx)); err != nil {
 		return nil, err
 	}
 
@@ -309,7 +309,11 @@ func makeIndexDescriptor(
 }
 
 func checkIndexColumns(
-	desc catalog.TableDescriptor, columns tree.IndexElemList, storing tree.NameList, inverted bool,
+	desc catalog.TableDescriptor,
+	columns tree.IndexElemList,
+	storing tree.NameList,
+	inverted bool,
+	version clusterversion.ClusterVersion,
 ) error {
 	for i, colDef := range columns {
 		col, err := catalog.MustFindColumnByTreeName(desc, colDef.Column)
@@ -326,6 +330,21 @@ func checkIndexColumns(
 			return pgerror.New(pgcode.DatatypeMismatch,
 				"operator classes are only allowed for the last column of an inverted index")
 		}
+
+		// Checking if JSON Columns can be forward indexed for a given cluster version.
+		if col.GetType().Family() == types.JsonFamily && !inverted &&
+			version.Less(clusterversion.ByKey(clusterversion.V23_2_JSONForwardIndexes)) {
+			return errors.WithHint(
+				pgerror.Newf(
+					pgcode.InvalidTableDefinition,
+					"index element %s of type %s is not indexable in a non-inverted index",
+					col.GetName(),
+					col.GetType().Name(),
+				),
+				"you may want to create an inverted index instead. See the documentation for inverted indexes: "+docs.URL("inverted-indexes.html"),
+			)
+		}
+
 	}
 	for i, colName := range storing {
 		col, err := catalog.MustFindColumnByTreeName(desc, colName)
@@ -534,6 +553,18 @@ func replaceExpressionElemsWithVirtualCols(
 				)
 			}
 
+			if typ.Family() == types.JsonFamily && version.Less(clusterversion.ByKey(clusterversion.V23_2_JSONForwardIndexes)) {
+				return errors.WithHint(
+					pgerror.Newf(
+						pgcode.InvalidTableDefinition,
+						"index element %s of type %s is not indexable in a non-inverted index",
+						elem.Expr.String(),
+						typ.Name(),
+					),
+					"you may want to create an inverted index instead. See the documentation for inverted indexes: "+docs.URL("inverted-indexes.html"),
+				)
+			}
+
 			if !isInverted && !colinfo.ColumnTypeIsIndexable(typ) {
 				if colinfo.ColumnTypeIsInvertedIndexable(typ) {
 					return errors.WithHint(

pkg/sql/create_table.go

@@ -19,6 +19,7 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/build"
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/docs"
 	"github.com/cockroachdb/cockroach/pkg/jobs"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
@@ -1601,6 +1602,20 @@ func NewTableDesc(
 				incTelemetryForNewColumn(d, col)
 			}
 
+			// Version gates for enabling primary keys for JSONB columns
+			if col.Type.Family() == types.JsonFamily && d.PrimaryKey.IsPrimaryKey &&
+				version.Less(clusterversion.ByKey(clusterversion.V23_2_JSONForwardIndexes)) {
+				return nil, errors.WithHint(
+					pgerror.Newf(
+						pgcode.InvalidTableDefinition,
+						"index element %s of type %s is not indexable in a non-inverted index",
+						col.Name,
+						col.Type.Name(),
+					),
+					"you may want to create an inverted index instead. See the documentation for inverted indexes: "+docs.URL("inverted-indexes.html"),
+				)
+			}
+
 			desc.AddColumn(col)
 
 			if idx != nil {
@@ -1785,7 +1800,7 @@ func NewTableDesc(
 			); err != nil {
 				return nil, err
 			}
-			if err := checkIndexColumns(&desc, d.Columns, d.Storing, d.Inverted); err != nil {
+			if err := checkIndexColumns(&desc, d.Columns, d.Storing, d.Inverted, version); err != nil {
 				return nil, err
 			}
 			idx := descpb.IndexDescriptor{

pkg/sql/logictest/testdata/logic_test/crdb_internal

@@ -569,7 +569,7 @@ select crdb_internal.get_vmodule()
 query T
 select regexp_replace(crdb_internal.node_executable_version()::string, '(-\d+)?$', '');
 ----
-1000023.1
+1000023.2
 
 query ITTT colnames
 select node_id, component, field, regexp_replace(regexp_replace(value, '^\d+$', '<port>'), e':\\d+', ':<port>') as value from crdb_internal.node_runtime_info
@@ -723,7 +723,7 @@ SELECT * FROM crdb_internal.check_consistency(true, b'\x02', b'\x04')
 query T
 select regexp_replace(crdb_internal.node_executable_version()::string, '(-\d+)?$', '');
 ----
-1000023.1
+1000023.2
 
 user root
 

pkg/sql/logictest/testdata/logic_test/expression_index

@@ -1,3 +1,5 @@
+# LogicTest: default-configs !local-mixed-22.2-23.1
+
 statement ok
 CREATE TABLE t (
   k INT PRIMARY KEY,

pkg/sql/logictest/testdata/logic_test/json

@@ -182,6 +182,7 @@ SELECT bar FROM foo WHERE bar->'a' = '"b"'::JSON
 statement ok
 SELECT bar FROM foo ORDER BY bar
 
+skipif config local-mixed-22.2-23.1
 statement ok
 CREATE TABLE pk (k JSON PRIMARY KEY)
 

pkg/sql/logictest/testdata/logic_test/json_index

@@ -1,3 +1,5 @@
+# LogicTest: default-configs !local-mixed-22.2-23.1
+
 # Add JSON columns as primary index.
 statement ok
 CREATE TABLE t (x JSONB PRIMARY KEY)

pkg/sql/logictest/testdata/logic_test/mixed_version_can_login

@@ -23,7 +23,7 @@ upgrade 1
 query B
 SELECT crdb_internal.node_executable_version() SIMILAR TO '1000023.1-%'
 ----
-true
+false
 
 query T nodeidx=2
 SELECT crdb_internal.node_executable_version()
@@ -36,15 +36,15 @@ user testuser nodeidx=0
 query B
 SELECT crdb_internal.node_executable_version() SIMILAR TO '1000023.1-%'
 ----
-true
+false
 
 # Verify that a root user can login on the upgraded node.
 user root nodeidx=1
 
 query B
 SELECT crdb_internal.node_executable_version() SIMILAR TO '1000023.1-%'
 ----
-true
+false
 
 # Verify that a non-root user can login on the non-upgraded node.
 user testuser nodeidx=2
@@ -67,7 +67,7 @@ upgrade 2
 query B
 SELECT crdb_internal.node_executable_version() SIMILAR TO '1000023.1-%'
 ----
-true
+false
 
 statement ok
 CREATE TABLE bootstrapped_tenant_data AS

pkg/sql/logictest/testdata/logic_test/mixed_version_range_tombstones

@@ -24,7 +24,7 @@ upgrade 0
 query B nodeidx=0
 SELECT crdb_internal.node_executable_version() SIMILAR TO '1000023.1-%'
 ----
-true
+false
 
 # Range tombstones remain disabled.
 

pkg/sql/logictest/testdata/logic_test/mixed_version_role_members_user_ids

@@ -49,7 +49,7 @@ upgrade 1
 query B nodeidx=1
 SELECT crdb_internal.node_executable_version() SIMILAR TO '1000023.1-%'
 ----
-true
+false
 
 user root nodeidx=1
 
@@ -85,14 +85,14 @@ upgrade 2
 query B nodeidx=0
 SELECT crdb_internal.node_executable_version() SIMILAR TO '1000023.1-%'
 ----
-true
+false
 
 query B nodeidx=1
 SELECT crdb_internal.node_executable_version() SIMILAR TO '1000023.1-%'
 ----
-true
+false
 
 query B nodeidx=2
 SELECT crdb_internal.node_executable_version() SIMILAR TO '1000023.1-%'
 ----
-true
+false

pkg/upgrade/upgrades/BUILD.bazel

@@ -108,6 +108,7 @@ go_test(
         "ensure_sql_schema_telemetry_schedule_test.go",
         "external_connections_table_user_id_migration_test.go",
         "helpers_test.go",
+        "json_forward_indexes_test.go",
         "key_visualizer_migration_test.go",
         "main_test.go",
         "role_members_ids_migration_test.go",