Skip to content

Commit

Permalink
sql: adding version gates for changes related to JSON forward indexes
Browse files Browse the repository at this point in the history
Previously, cockroachdb#99275 and cockroachdb#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: cockroachdb#35706
  • Loading branch information
Shivs11 committed Apr 24, 2023
1 parent cfa9ede commit e287ba9
Show file tree
Hide file tree
Showing 12 changed files with 178 additions and 19 deletions.
2 changes: 1 addition & 1 deletion docs/generated/settings/settings-for-tenants.txt
Expand Up @@ -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
2 changes: 1 addition & 1 deletion docs/generated/settings/settings.html
Expand Up @@ -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>
8 changes: 8 additions & 0 deletions pkg/clusterversion/cockroach_versions.go
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
35 changes: 33 additions & 2 deletions pkg/sql/create_index.go
Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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(
Expand Down
17 changes: 16 additions & 1 deletion pkg/sql/create_table.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
Expand Down
2 changes: 2 additions & 0 deletions 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,
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/logictest/testdata/logic_test/json
Expand Up @@ -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)

Expand Down
2 changes: 2 additions & 0 deletions 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)
Expand Down
14 changes: 0 additions & 14 deletions pkg/sql/logictest/tests/local-mixed-22.2-23.1/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/upgrade/upgrades/BUILD.bazel
Expand Up @@ -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",
Expand Down
107 changes: 107 additions & 0 deletions pkg/upgrade/upgrades/json_forward_indexes_test.go
@@ -0,0 +1,107 @@
// 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)

// Updating the cluster version to allow creation of forward indexes
// on JSON columns.
_, err = tc.Conns[0].ExecContext(ctx, `SET CLUSTER SETTING version = $1`,
clusterversion.ByKey(clusterversion.V23_2_JSONForwardIndexes).String())
require.NoError(t, err)

_, err = db.Exec(`CREATE INDEX on test.hello (j)`)
require.NoError(t, err)

_, err = db.Exec(`INSERT INTO test.hello VALUES (1, '[1, 2, 3]'::JSONB)`)
require.NoError(t, err)

_, err = db.Exec(`CREATE INDEX on test.hello ((j->'a'))`)
require.NoError(t, err)

_, err = db.Exec(`INSERT INTO test.hello VALUES (2, '{"a": "b"}'::JSONB)`)
require.NoError(t, err)

// Creating a primary key on a JSON column.
_, err = db.Exec(`CREATE TABLE test.new (
key JSONB PRIMARY KEY
)`)
require.NoError(t, err)
}
6 changes: 6 additions & 0 deletions pkg/upgrade/upgrades/upgrades.go
Expand Up @@ -310,6 +310,12 @@ var upgrades = []upgradebase.Upgrade{
createActivityUpdateJobMigration,
"create statement_activity and transaction_activity job",
),
upgrade.NewTenantUpgrade(
"enable JSON forward indexes and ORDER BY on JSON columns",
toCV(clusterversion.V23_2_JSONForwardIndexes),
upgrade.NoPrecondition,
NoTenantUpgradeFunc,
),
}

func init() {
Expand Down

0 comments on commit e287ba9

Please sign in to comment.