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.1-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.1-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/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,20 @@ 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.IsActive(clusterversion.V23_2) {
+			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 +552,18 @@ func replaceExpressionElemsWithVirtualCols(
 				)
 			}
 
+			if typ.Family() == types.JsonFamily && !version.IsActive(clusterversion.V23_2) {
+				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,19 @@ func NewTableDesc(
 				incTelemetryForNewColumn(d, col)
 			}
 
+			// Version gates for enabling primary keys for JSONB columns
+			if col.Type.Family() == types.JsonFamily && d.PrimaryKey.IsPrimaryKey && !version.IsActive(clusterversion.V23_2) {
+				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 +1799,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/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/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",

pkg/upgrade/upgrades/json_forward_indexes_test.go

@@ -0,0 +1,84 @@
+// 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 upgrades_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/base"
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
+	"github.com/cockroachdb/cockroach/pkg/server"
+	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
+	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
+	"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
+	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/stretchr/testify/require"
+)
+
+func TestJSONForwardingIndexes(t *testing.T) {
+	var err error
+	skip.UnderStressRace(t)
+	defer leaktest.AfterTest(t)()
+	ctx := context.Background()
+
+	settings := cluster.MakeTestingClusterSettingsWithVersions(
+		clusterversion.TestingBinaryVersion,
+		clusterversion.TestingBinaryMinSupportedVersion,
+		false,
+	)
+
+	tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{
+		ServerArgs: base.TestServerArgs{
+			Settings: settings,
+			Knobs: base.TestingKnobs{
+				Server: &server.TestingKnobs{
+					DisableAutomaticVersionUpgrade: make(chan struct{}),
+					BinaryVersionOverride:          clusterversion.TestingBinaryMinSupportedVersion,
+				},
+			},
+		},
+	})
+	defer tc.Stopper().Stop(ctx)
+
+	db := tc.ServerConn(0)
+	defer db.Close()
+
+	// Setting an older cluster version and expecting an error when creating
+	// forward indexes on JSON columns.
+	_, err = tc.Conns[0].ExecContext(ctx, `SET CLUSTER SETTING version = $1`,
+		clusterversion.ByKey(clusterversion.V23_1WebSessionsTableHasUserIDColumn).String())
+	require.NoError(t, err)
+
+	_, err = db.Exec(`CREATE DATABASE test`)
+	require.NoError(t, err)
+
+	_, err = db.Exec(`CREATE TABLE test.hello (
+    key INT PRIMARY KEY,
+    j JSONB
+)`)
+	require.NoError(t, err)
+
+	// Creating an index on the JSON column should result in an error.
+	_, err = db.Exec(`CREATE INDEX on test.hello (j)`)
+	require.Error(t, err)
+
+	// Creating an JSON expression index should result in an error.
+	_, err = db.Exec(`CREATE INDEX on test.hello ((j->'a'))`)
+	require.Error(t, err)
+
+	// Creating a primary key on a JSON column should result in an error.
+	_, err = db.Exec(`CREATE TABLE test.new (
+	   key JSONB PRIMARY KEY
+	)`)
+	require.Error(t, err)
+
+}