Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
59835: colrpc: add a comment about runnerCtx check in sendBatches r=yuzefovich a=yuzefovich

Release note: None

60018: ccl: add inverted indexes to regional by row tests r=mgartner a=mgartner

Release note: None

60159: sql: explicitly verify descriptors are sequences r=the-ericwang35 a=the-ericwang35

Fixes #59991.

Previously, we relied on ObjectLookupFlags to verify descriptor types.
This was inadequate because ID based resolution ignores the flag and
doesn't actually make the check. This patch adds an explicit check to verify
the returned descriptor is a sequence.

Release note (bug fix): explicitly verify descriptors are sequences

Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Eric Wang <[email protected]>
  • Loading branch information
4 people committed Feb 9, 2021
4 parents 4086c3e + 9352005 + 5cd729c + 53b7d3f commit 0cccaab
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 26 deletions.
97 changes: 71 additions & 26 deletions pkg/ccl/logictestccl/testdata/logic_test/regional_by_row
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ CREATE TABLE regional_by_row_table (
)
LOCALITY REGIONAL BY ROW

statement error REGIONAL BY ROW on a table with an INDEX containing PARTITION BY is not supported
CREATE TABLE regional_by_row_table (
pk int,
a int,
j JSON,
INVERTED INDEX (a, j) PARTITION BY LIST (a) (PARTITION one VALUES IN ((1)))
)
LOCALITY REGIONAL BY ROW

statement error REGIONAL BY ROW on a table with an UNIQUE constraint containing PARTITION BY is not supported
CREATE TABLE regional_by_row_table (
pk int,
Expand All @@ -76,8 +85,10 @@ CREATE TABLE regional_by_row_table (
pk2 int NOT NULL,
a int NOT NULL,
b int NOT NULL,
j JSON,
INDEX (a),
UNIQUE (b),
INVERTED INDEX (j),
FAMILY (pk, pk2, a, b)
) LOCALITY REGIONAL BY ROW

Expand All @@ -90,11 +101,13 @@ CREATE TABLE public.regional_by_row_table (
pk2 INT8 NOT NULL,
a INT8 NOT NULL,
b INT8 NOT NULL,
j JSONB NULL,
crdb_region public.crdb_internal_region NOT VISIBLE NOT NULL DEFAULT gateway_region()::public.crdb_internal_region,
CONSTRAINT "primary" PRIMARY KEY (pk ASC),
INDEX regional_by_row_table_a_idx (a ASC),
UNIQUE INDEX regional_by_row_table_b_key (b ASC),
FAMILY fam_0_pk_pk2_a_b_crdb_region (pk, pk2, a, b, crdb_region)
INVERTED INDEX regional_by_row_table_j_idx (j),
FAMILY fam_0_pk_pk2_a_b_j_crdb_region (pk, pk2, a, b, j, crdb_region)
) LOCALITY REGIONAL BY ROW;
ALTER PARTITION "ap-southeast-2" OF INDEX multi_region_test_db.public.regional_by_row_table@primary CONFIGURE ZONE USING
num_voters = 5,
Expand All @@ -108,6 +121,10 @@ ALTER PARTITION "ap-southeast-2" OF INDEX multi_region_test_db.public.regional_b
num_voters = 5,
voter_constraints = '{+region=ap-southeast-2: 2}',
lease_preferences = '[[+region=ap-southeast-2]]';
ALTER PARTITION "ap-southeast-2" OF INDEX multi_region_test_db.public.regional_by_row_table@regional_by_row_table_j_idx CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=ap-southeast-2: 2}',
lease_preferences = '[[+region=ap-southeast-2]]';
ALTER PARTITION "ca-central-1" OF INDEX multi_region_test_db.public.regional_by_row_table@primary CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=ca-central-1: 2}',
Expand All @@ -120,6 +137,10 @@ ALTER PARTITION "ca-central-1" OF INDEX multi_region_test_db.public.regional_by_
num_voters = 5,
voter_constraints = '{+region=ca-central-1: 2}',
lease_preferences = '[[+region=ca-central-1]]';
ALTER PARTITION "ca-central-1" OF INDEX multi_region_test_db.public.regional_by_row_table@regional_by_row_table_j_idx CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=ca-central-1: 2}',
lease_preferences = '[[+region=ca-central-1]]';
ALTER PARTITION "us-east-1" OF INDEX multi_region_test_db.public.regional_by_row_table@primary CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=us-east-1: 2}',
Expand All @@ -129,6 +150,10 @@ ALTER PARTITION "us-east-1" OF INDEX multi_region_test_db.public.regional_by_row
voter_constraints = '{+region=us-east-1: 2}',
lease_preferences = '[[+region=us-east-1]]';
ALTER PARTITION "us-east-1" OF INDEX multi_region_test_db.public.regional_by_row_table@regional_by_row_table_b_key CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=us-east-1: 2}',
lease_preferences = '[[+region=us-east-1]]';
ALTER PARTITION "us-east-1" OF INDEX multi_region_test_db.public.regional_by_row_table@regional_by_row_table_j_idx CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=us-east-1: 2}',
lease_preferences = '[[+region=us-east-1]]'
Expand All @@ -145,6 +170,8 @@ regional_by_row_table_a_idx a false
regional_by_row_table_a_idx crdb_region true
regional_by_row_table_b_key b false
regional_by_row_table_b_key crdb_region true
regional_by_row_table_j_idx crdb_region true
regional_by_row_table_j_idx j false

query TTTTIT colnames
SHOW TABLES
Expand All @@ -153,8 +180,8 @@ schema_name table_name type owner estimated_row_count locality
public regional_by_row_table table root 0 REGIONAL BY ROW

query TI
INSERT INTO regional_by_row_table (pk, pk2, a, b) VALUES
(1, 1, 2, 3), (4, 4, 5, 6)
INSERT INTO regional_by_row_table (pk, pk2, a, b, j) VALUES
(1, 1, 2, 3, '{"a": "b"}'), (4, 4, 5, 6, '{"c": "d"}')
RETURNING crdb_region, pk
----
ap-southeast-2 1
Expand Down Expand Up @@ -199,16 +226,16 @@ ca-central-1 10 10 11 12
us-east-1 20 20 21 22
ap-southeast-2 23 23 24 25

query IIII colnames
query IIIIT colnames
SELECT * FROM regional_by_row_table ORDER BY pk
----
pk pk2 a b
1 1 2 3
4 4 5 6
7 7 8 9
10 10 11 12
20 20 21 22
23 23 24 25
pk pk2 a b j
1 1 2 3 {"a": "b"}
4 4 5 6 {"c": "d"}
7 7 8 9 NULL
10 10 11 12 NULL
20 20 21 22 NULL
23 23 24 25 NULL

# Tests creating a index and a unique constraint on a REGIONAL BY ROW table.
statement ok
Expand All @@ -228,23 +255,23 @@ vectorized: true
• root
├── • insert
│ │ into: regional_by_row_table(pk, pk2, a, b, crdb_region)
│ │ into: regional_by_row_table(pk, pk2, a, b, j, crdb_region)
│ │
│ └── • buffer
│ │ label: buffer 1
│ │
│ └── • values
│ size: 6 columns, 1 row
│ size: 7 columns, 1 row
├── • constraint-check
│ │
│ └── • error if rows
│ │
│ └── • lookup join (semi)
│ │ table: regional_by_row_table@primary
│ │ equality: (lookup_join_const_col_@20, column1) = (crdb_region,pk)
│ │ equality: (lookup_join_const_col_@24, column1) = (crdb_region,pk)
│ │ equality cols are key
│ │ pred: column12 != crdb_region
│ │ pred: column15 != crdb_region
│ │
│ └── • cross join
│ │
Expand All @@ -260,9 +287,9 @@ vectorized: true
│ │
│ └── • lookup join (semi)
│ │ table: regional_by_row_table@regional_by_row_table_b_key
│ │ equality: (lookup_join_const_col_@30, column4) = (crdb_region,b)
│ │ equality: (lookup_join_const_col_@36, column4) = (crdb_region,b)
│ │ equality cols are key
│ │ pred: (column1 != pk) OR (column12 != crdb_region)
│ │ pred: (column1 != pk) OR (column15 != crdb_region)
│ │
│ └── • cross join
│ │
Expand All @@ -278,9 +305,9 @@ vectorized: true
└── • lookup join (semi)
│ table: regional_by_row_table@new_idx
│ equality: (lookup_join_const_col_@41, column3, column4) = (crdb_region,a,b)
│ equality: (lookup_join_const_col_@49, column3, column4) = (crdb_region,a,b)
│ equality cols are key
│ pred: (column1 != pk) OR (column12 != crdb_region)
│ pred: (column1 != pk) OR (column15 != crdb_region)
└── • cross join
Expand All @@ -304,7 +331,7 @@ vectorized: true
• root
├── • upsert
│ │ into: regional_by_row_table(pk, pk2, a, b, crdb_region)
│ │ into: regional_by_row_table(pk, pk2, a, b, j, crdb_region)
│ │ arbiter constraints: primary
│ │
│ └── • buffer
Expand All @@ -320,15 +347,15 @@ vectorized: true
│ │ spans: [/'ap-southeast-2'/2 - /'ap-southeast-2'/2] [/'ca-central-1'/2 - /'ca-central-1'/2] [/'us-east-1'/2 - /'us-east-1'/2]
│ │
│ └── • values
│ size: 5 columns, 1 row
│ size: 6 columns, 1 row
├── • constraint-check
│ │
│ └── • error if rows
│ │
│ └── • lookup join (semi)
│ │ table: regional_by_row_table@primary
│ │ equality: (lookup_join_const_col_@28, upsert_pk) = (crdb_region,pk)
│ │ equality: (lookup_join_const_col_@35, upsert_pk) = (crdb_region,pk)
│ │ equality cols are key
│ │ pred: column1 != crdb_region
│ │
Expand All @@ -346,7 +373,7 @@ vectorized: true
│ │
│ └── • lookup join (semi)
│ │ table: regional_by_row_table@regional_by_row_table_b_key
│ │ equality: (lookup_join_const_col_@38, column5) = (crdb_region,b)
│ │ equality: (lookup_join_const_col_@47, column5) = (crdb_region,b)
│ │ equality cols are key
│ │ pred: (upsert_pk != pk) OR (column1 != crdb_region)
│ │
Expand All @@ -364,7 +391,7 @@ vectorized: true
└── • lookup join (semi)
│ table: regional_by_row_table@new_idx
│ equality: (lookup_join_const_col_@49, column4, column5) = (crdb_region,a,b)
│ equality: (lookup_join_const_col_@60, column4, column5) = (crdb_region,a,b)
│ equality cols are key
│ pred: (upsert_pk != pk) OR (column1 != crdb_region)
Expand Down Expand Up @@ -401,13 +428,15 @@ CREATE TABLE public.regional_by_row_table (
pk2 INT8 NOT NULL,
a INT8 NOT NULL,
b INT8 NOT NULL,
j JSONB NULL,
crdb_region public.crdb_internal_region NOT VISIBLE NOT NULL DEFAULT gateway_region()::public.crdb_internal_region,
CONSTRAINT "primary" PRIMARY KEY (pk ASC),
INDEX regional_by_row_table_a_idx (a ASC),
UNIQUE INDEX regional_by_row_table_b_key (b ASC),
INVERTED INDEX regional_by_row_table_j_idx (j),
INDEX new_idx (a ASC, b ASC),
UNIQUE INDEX unique_b_a (b ASC, a ASC),
FAMILY fam_0_pk_pk2_a_b_crdb_region (pk, pk2, a, b, crdb_region)
FAMILY fam_0_pk_pk2_a_b_j_crdb_region (pk, pk2, a, b, j, crdb_region)
) LOCALITY REGIONAL BY ROW;
ALTER PARTITION "ap-southeast-2" OF INDEX multi_region_test_db.public.regional_by_row_table@new_idx CONFIGURE ZONE USING
num_voters = 5,
Expand All @@ -425,6 +454,10 @@ ALTER PARTITION "ap-southeast-2" OF INDEX multi_region_test_db.public.regional_b
num_voters = 5,
voter_constraints = '{+region=ap-southeast-2: 2}',
lease_preferences = '[[+region=ap-southeast-2]]';
ALTER PARTITION "ap-southeast-2" OF INDEX multi_region_test_db.public.regional_by_row_table@regional_by_row_table_j_idx CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=ap-southeast-2: 2}',
lease_preferences = '[[+region=ap-southeast-2]]';
ALTER PARTITION "ap-southeast-2" OF INDEX multi_region_test_db.public.regional_by_row_table@unique_b_a CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=ap-southeast-2: 2}',
Expand All @@ -445,6 +478,10 @@ ALTER PARTITION "ca-central-1" OF INDEX multi_region_test_db.public.regional_by_
num_voters = 5,
voter_constraints = '{+region=ca-central-1: 2}',
lease_preferences = '[[+region=ca-central-1]]';
ALTER PARTITION "ca-central-1" OF INDEX multi_region_test_db.public.regional_by_row_table@regional_by_row_table_j_idx CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=ca-central-1: 2}',
lease_preferences = '[[+region=ca-central-1]]';
ALTER PARTITION "ca-central-1" OF INDEX multi_region_test_db.public.regional_by_row_table@unique_b_a CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=ca-central-1: 2}',
Expand All @@ -465,6 +502,10 @@ ALTER PARTITION "us-east-1" OF INDEX multi_region_test_db.public.regional_by_row
num_voters = 5,
voter_constraints = '{+region=us-east-1: 2}',
lease_preferences = '[[+region=us-east-1]]';
ALTER PARTITION "us-east-1" OF INDEX multi_region_test_db.public.regional_by_row_table@regional_by_row_table_j_idx CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=us-east-1: 2}',
lease_preferences = '[[+region=us-east-1]]';
ALTER PARTITION "us-east-1" OF INDEX multi_region_test_db.public.regional_by_row_table@unique_b_a CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=us-east-1: 2}',
Expand All @@ -485,6 +526,8 @@ regional_by_row_table_a_idx a false
regional_by_row_table_a_idx crdb_region true
regional_by_row_table_b_key b false
regional_by_row_table_b_key crdb_region true
regional_by_row_table_j_idx crdb_region true
regional_by_row_table_j_idx j false
unique_b_a a false
unique_b_a b false
unique_b_a crdb_region true
Expand Down Expand Up @@ -516,14 +559,16 @@ CREATE TABLE public.regional_by_row_table (
pk2 INT8 NOT NULL,
a INT8 NOT NULL,
b INT8 NOT NULL,
j JSONB NULL,
crdb_region public.crdb_internal_region NOT VISIBLE NOT NULL DEFAULT gateway_region()::public.crdb_internal_region,
CONSTRAINT "primary" PRIMARY KEY (pk2 ASC),
UNIQUE INDEX regional_by_row_table_pk_key (pk ASC),
INDEX regional_by_row_table_a_idx (a ASC),
UNIQUE INDEX regional_by_row_table_b_key (b ASC),
INVERTED INDEX regional_by_row_table_j_idx (j),
INDEX new_idx (a ASC, b ASC),
UNIQUE INDEX unique_b_a (b ASC, a ASC),
FAMILY fam_0_pk_pk2_a_b_crdb_region (pk, pk2, a, b, crdb_region)
FAMILY fam_0_pk_pk2_a_b_j_crdb_region (pk, pk2, a, b, j, crdb_region)
) LOCALITY REGIONAL BY ROW

query TT
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/colflow/colrpc/outbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,9 @@ func (o *Outbox) sendBatches(
ctx context.Context, stream flowStreamClient, cancelFn context.CancelFunc,
) (terminatedGracefully bool, errToSend error) {
if o.runnerCtx == nil {
// In the non-testing path, runnerCtx has been set in Run() method;
// however, the tests might use runWithStream() directly in which case
// runnerCtx will remain unset, so we have this check.
o.runnerCtx = ctx
}
errToSend = colexecerror.CatchVectorizedRuntimeError(func() {
Expand Down
89 changes: 89 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/sequences
Original file line number Diff line number Diff line change
Expand Up @@ -1394,3 +1394,92 @@ CREATE SEQUENCE db2.seq2 OWNED BY db1.t.a

statement error invalid OWNED BY option
ALTER SEQUENCE db2.seq2 OWNED BY doesntexist


# Test that passing a descriptor other than a sequence
# returns an appropriate error.
subtest invalid_ids

statement ok
CREATE TABLE t (i SERIAL PRIMARY KEY)

let $t_id
SELECT 't'::regclass::int

statement error pgcode 42809 "test.public.t" is not a sequence
SELECT nextval($t_id::regclass)

statement error pgcode 42809 "test.public.t" is not a sequence
SELECT setval($t_id::regclass, 30, false)

statement error pgcode 42809 "test.public.t" is not a sequence
SELECT currval($t_id::regclass)

statement ok
CREATE VIEW v AS (SELECT i FROM t)

let $v_id
SELECT 'v'::regclass::int

statement error pgcode 42809 "test.public.v" is not a sequence
SELECT nextval($v_id::regclass)

statement error pgcode 42809 "test.public.v" is not a sequence
SELECT setval($v_id::regclass, 30, false)

statement error pgcode 42809 "test.public.v" is not a sequence
SELECT currval($v_id::regclass)

statement ok
CREATE SCHEMA sc

let $sc_id
SELECT id FROM system.namespace WHERE name='sc'

statement error does not exist
SELECT nextval($sc_id::regclass)

statement error does not exist
SELECT setval($sc_id::regclass, 30, false)

statement error does not exist
SELECT currval($sc_id::regclass)

statement ok
CREATE DATABASE db

let $db_id
SELECT id FROM system.namespace WHERE name='db'

statement error does not exist
SELECT nextval($db_id::regclass)

statement error does not exist
SELECT setval($db_id::regclass, 30, false)

statement error does not exist
SELECT currval($db_id::regclass)

statement ok
CREATE TYPE e AS ENUM ('foo', 'bar')

let $e_id
SELECT id FROM system.namespace WHERE name='e'

statement error does not exist
SELECT nextval($e_id::regclass)

statement error does not exist
SELECT setval($e_id::regclass, 30, false)

statement error does not exist
SELECT currval($e_id::regclass)

statement error does not exist
SELECT nextval(12345::regclass) # Bogus ID

statement error does not exist
SELECT setval(12345::regclass, 30, false) # Bogus ID

statement error does not exist
SELECT currval(12345::regclass) # Bogus ID
Loading

0 comments on commit 0cccaab

Please sign in to comment.