Skip to content

Commit

Permalink
sql: version gate JSON columns on inverted indexes
Browse files Browse the repository at this point in the history
Previously, our version gate for forward JSON indexes
did not correctly handle the case when they are used
in inverted indexes. When these columns are not the last
column in an inverted index they need to be forward indexable,
requiring a similar version gate. This patch fixes version
gate logic for this scenario.

Fixes: cockroachdb#104178, cockroachdb#104707
Release note: None
  • Loading branch information
fqazi committed Jul 4, 2023
1 parent 4104f66 commit 31b790e
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 1 deletion.
3 changes: 2 additions & 1 deletion pkg/sql/create_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ func checkIndexColumns(
version clusterversion.ClusterVersion,
) error {
for i, colDef := range columns {
lastCol := i == len(columns)-1
col, err := catalog.MustFindColumnByTreeName(desc, colDef.Column)
if err != nil {
return errors.Wrapf(err, "finding column %d", i)
Expand All @@ -342,7 +343,7 @@ func checkIndexColumns(
}

// 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) {
if col.GetType().Family() == types.JsonFamily && (!inverted || !lastCol) && !version.IsActive(clusterversion.V23_2) {
return errors.WithHint(
pgerror.Newf(
pgcode.InvalidTableDefinition,
Expand Down
15 changes: 15 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/inverted_index_multi_column
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ SELECT * FROM backfill_d@idx WHERE i IN (1, 2, 3, 4) AND s = 'bar' AND j @> '7':

# Test selecting, inserting, updating, and deleting on a table with a
# multi-column JSON inverted index.
skipif config local-mixed-22.2-23.1
statement ok
CREATE TABLE d (
id INT PRIMARY KEY,
Expand All @@ -331,6 +332,17 @@ CREATE TABLE d (
INVERTED INDEX idx (foo, bar)
);

# On 22.2 these inverted indexes could not be created, so skip this
# test by making a normal index
onlyif config local-mixed-22.2-23.1
statement ok
CREATE TABLE d (
id INT PRIMARY KEY,
foo JSONB,
bar JSONB,
INDEX idx (id)
);

# Testing inserting
statement ok
INSERT into d VALUES
Expand Down Expand Up @@ -394,15 +406,18 @@ INSERT into d VALUES
(8, '"backfilling"', '[[0], [1], [2], []]')


skipif config local-mixed-22.2-23.1
statement ok
CREATE INVERTED INDEX idx on d (foo, bar)

skipif config local-mixed-22.2-23.1
query ITT
SELECT id, foo, bar FROM d@idx where foo = '"backfilling"' AND bar->2 @> '2' ORDER BY id
----
6 "backfilling" [[0], [1], 2, 3]
8 "backfilling" [[0], [1], [2], []]

skipif config local-mixed-22.2-23.1
query ITT
SELECT id, foo, bar FROM d@idx where foo = '"foo"' AND bar->0 = '7' ORDER BY id
----
Expand Down
17 changes: 17 additions & 0 deletions pkg/upgrade/upgrades/json_forward_indexes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,14 @@ func TestJSONForwardingIndexes(t *testing.T) {
)`)
require.Error(t, err)

// Creating a table with an inverted index with a JSON column should result in an error.
_, err = db.Exec(`CREATE TABLE test.tbl_json_secondary_unique(
s GEOGRAPHY,
j JSONB,
INVERTED INDEX tbl_json_secondary_uidx (j, s)
)`)
require.Error(t, err)

// Setting a cluster version that supports forward indexes on JSON
// columns and expecting success when creating forward indexes.
_, err = tc.Conns[0].ExecContext(ctx, `SET CLUSTER SETTING version = $1`,
Expand All @@ -158,4 +166,13 @@ func TestJSONForwardingIndexes(t *testing.T) {
key JSONB PRIMARY KEY
)`)
require.NoError(t, err)

// Creating a table with an inverted index with a JSON column should not result
// in an error once inverted indexes are supported.
_, err = db.Exec(`CREATE TABLE test.tbl_json_secondary_unique(
s GEOGRAPHY,
j JSONB,
INVERTED INDEX tbl_json_secondary_uidx (j, s)
)`)
require.NoError(t, err)
}

0 comments on commit 31b790e

Please sign in to comment.