Skip to content

Commit

Permalink
Merge #61451 #61525
Browse files Browse the repository at this point in the history
61451: sql: block PARTITION BY on multi-region tables r=ajstorm a=otan

See individual commits for details.

Note that this does not cover SET PRIMARY REGION yet.

Resolves #59719

61525: geomfn: simplify InEpsilon checks in tests r=andyyang890 a=otan

These checks were used repeatedly and copy-pasta'd, why not simplify
them into the one.

Release justification: non-production code change

Release note: None

Co-authored-by: Oliver Tan <[email protected]>
  • Loading branch information
craig[bot] and otan committed Mar 5, 2021
3 parents 9ccda03 + a6fd9d8 + 49a6b27 commit f02e38f
Show file tree
Hide file tree
Showing 20 changed files with 361 additions and 188 deletions.
7 changes: 6 additions & 1 deletion pkg/ccl/importccl/import_table_creation.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,14 +164,19 @@ func MakeSimpleTableDescriptor(
parentID,
parentSchemaID,
tableID,
descpb.InvalidID,
nil, /* regionConfig */
hlc.Timestamp{WallTime: walltime},
descpb.NewDefaultPrivilegeDescriptor(security.AdminRoleName()),
affected,
semaCtx,
&evalCtx,
&sessiondata.SessionData{}, /* sessionData */
tree.PersistencePermanent,
// We need to bypass the LOCALITY on non multi-region check here because
// we cannot access the database region config at import level.
// There is code that only allows REGIONAL BY TABLE tables to be imported,
// which will safely execute even if the locality check is bypassed.
sql.NewTableDescOptionBypassLocalityOnNonMultiRegionDatabaseCheck(),
)
if err != nil {
return nil, err
Expand Down
112 changes: 107 additions & 5 deletions pkg/ccl/logictestccl/testdata/logic_test/multi_region
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ ap-southeast-2 {ap-az1,ap-az2,ap-az3}
ca-central-1 {ca-az1,ca-az2,ca-az3}
us-east-1 {us-az1,us-az2,us-az3}

statement error cannot set LOCALITY on a table in a database that is not multi-region enabled
CREATE TABLE regional_by_table_table (pk int) LOCALITY REGIONAL BY TABLE

statement error cannot set LOCALITY on a table in a database that is not multi-region enabled
CREATE TABLE global_table (pk int) LOCALITY GLOBAL

statement ok
CREATE DATABASE region_test_db PRIMARY REGION "ap-southeast-2" SURVIVE ZONE FAILURE

Expand Down Expand Up @@ -84,10 +90,10 @@ USE multi_region_test_db
query IITI colnames
SELECT * FROM system.namespace WHERE name='crdb_internal_region'
----
parentID parentSchemaID name id
53 29 crdb_internal_region 54
56 29 crdb_internal_region 57
59 29 crdb_internal_region 60
parentID parentSchemaID name id
55 29 crdb_internal_region 56
58 29 crdb_internal_region 59
61 29 crdb_internal_region 62

query TTTT colnames
SHOW ENUMS FROM region_test_db.public
Expand Down Expand Up @@ -276,6 +282,82 @@ CREATE DATABASE invalid_region_db PRIMARY REGION "region_no_exists"
statement error region "ap-southeast-2" defined multiple times
CREATE DATABASE duplicate_region_name_db PRIMARY REGION "ap-southeast-2" REGIONS "ap-southeast-2", "ap-southeast-2"

statement error multi-region tables containing PARTITION BY are not supported
CREATE TABLE partition_by_table_in_mr_database (
id INT PRIMARY KEY
)
PARTITION BY LIST (id) (
PARTITION "one" VALUES IN (1)
)

statement error multi-region tables containing PARTITION BY are not supported
CREATE TABLE partition_by_table_in_mr_database (
id INT PRIMARY KEY
)
PARTITION BY LIST (id) (
PARTITION "one" VALUES IN (1)
)
LOCALITY GLOBAL

statement error multi-region tables containing PARTITION BY are not supported
CREATE TABLE partition_by_table_in_mr_database (
id INT PRIMARY KEY
)
PARTITION BY LIST (id) (
PARTITION "one" VALUES IN (1)
)
LOCALITY REGIONAL BY TABLE

statement error multi-region tables with an INDEX containing PARTITION BY are not supported
CREATE TABLE partition_by_table_in_mr_database (
id INT PRIMARY KEY,
INDEX idx(id) PARTITION BY LIST (id) (
PARTITION "one" VALUES IN (1)
)
)

statement error multi-region tables with an INDEX containing PARTITION BY are not supported
CREATE TABLE partition_by_table_in_mr_database (
id INT PRIMARY KEY,
INDEX idx(id) PARTITION BY LIST (id) (
PARTITION "one" VALUES IN (1)
)
) LOCALITY GLOBAL

statement error multi-region tables with an INDEX containing PARTITION BY are not supported
CREATE TABLE partition_by_table_in_mr_database (
id INT PRIMARY KEY,
INDEX idx(id) PARTITION BY LIST (id) (
PARTITION "one" VALUES IN (1)
)
) LOCALITY REGIONAL BY TABLE

statement error multi-region tables with an UNIQUE constraint containing PARTITION BY are not supported
CREATE TABLE partition_by_table_in_mr_database (
id INT PRIMARY KEY,
UNIQUE INDEX idx(id) PARTITION BY LIST (id) (
PARTITION "one" VALUES IN (1)
)
)

statement error multi-region tables with an UNIQUE constraint containing PARTITION BY are not supported
CREATE TABLE partition_by_table_in_mr_database (
id INT PRIMARY KEY,
UNIQUE INDEX idx(id) PARTITION BY LIST (id) (
PARTITION "one" VALUES IN (1)
)
)
LOCALITY GLOBAL

statement error multi-region tables with an UNIQUE constraint containing PARTITION BY are not supported
CREATE TABLE partition_by_table_in_mr_database (
id INT PRIMARY KEY,
UNIQUE INDEX idx(id) PARTITION BY LIST (id) (
PARTITION "one" VALUES IN (1)
)
)
LOCALITY REGIONAL BY TABLE

statement ok
CREATE TABLE regional_primary_region_table (a int) LOCALITY REGIONAL BY TABLE IN PRIMARY REGION

Expand All @@ -302,6 +384,16 @@ DATABASE multi_region_test_db ALTER DATABASE multi_region_test_db CONFIGURE ZON
voter_constraints = '{+region=ca-central-1: 2}',
lease_preferences = '[[+region=ca-central-1]]'

statement error cannot set PARTITION BY on a table in a multi-region enabled database
ALTER TABLE regional_primary_region_table PARTITION BY LIST (id) (
PARTITION "one" VALUES IN (1)
)

statement error cannot define PARTITION BY on a new INDEX in a multi-region database
CREATE INDEX bad_idx ON regional_primary_region_table(a) PARTITION BY LIST (a) (
PARTITION one VALUES IN (1)
)

statement ok
CREATE TABLE regional_implicit_primary_region_table (a int) LOCALITY REGIONAL BY TABLE

Expand Down Expand Up @@ -384,6 +476,16 @@ TABLE global_table ALTER TABLE global_table CONFIGURE ZONE USING
voter_constraints = '{+region=ca-central-1: 2}',
lease_preferences = '[[+region=ca-central-1]]'

statement error cannot set PARTITION BY on a table in a multi-region enabled database
ALTER TABLE global_table PARTITION BY LIST (id) (
PARTITION "one" VALUES IN (1)
)

statement error cannot define PARTITION BY on a new INDEX in a multi-region database
CREATE INDEX bad_idx ON global_table(a) PARTITION BY LIST (a) (
PARTITION one VALUES IN (1)
)

query TTTTIT colnames
SHOW TABLES
----
Expand All @@ -399,7 +501,7 @@ CREATE DATABASE new_db
statement ok
USE new_db

statement error database new_db is not multi-region enabled, but table cannot_create_table_no_multiregion has locality GLOBAL set
statement error cannot set LOCALITY on a table in a database that is not multi-region enabled
CREATE TABLE cannot_create_table_no_multiregion (a int) LOCALITY GLOBAL

# Test adding a primary region to the system database.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ ALTER DATABASE d PRIMARY REGION "us-east1"
statement error database must have associated regions before a survival goal can be set\nHINT: you must first add a primary region to the database using ALTER DATABASE d PRIMARY REGION <region_name>
ALTER DATABASE d SURVIVE ZONE FAILURE

statement error database test is not multi-region enabled, but table t has locality REGIONAL BY TABLE IN PRIMARY REGION set
statement error cannot set LOCALITY on a table in a database that is not multi-region enabled
CREATE TABLE t () LOCALITY REGIONAL BY TABLE

statement ok
Expand Down
31 changes: 25 additions & 6 deletions pkg/ccl/logictestccl/testdata/logic_test/partitioning_implicit
Original file line number Diff line number Diff line change
Expand Up @@ -293,22 +293,36 @@ CREATE TABLE fk_including_implicit_columns_against_t (
CONSTRAINT a_pk FOREIGN KEY(ref_t_a, ref_t_pk) REFERENCES t(a, pk)
)

statement error cannot ALTER TABLE PARTITION BY on table which already has implicit column partitioning
statement error cannot ALTER TABLE PARTITION BY on a table which already has implicit column partitioning
ALTER TABLE t PARTITION BY LIST(d) (
PARTITION pk_implicit VALUES IN (1)
)

statement error cannot ALTER INDEX PARTITION BY on index which already has implicit column partitioning
statement error cannot ALTER INDEX PARTITION BY on an index which already has implicit column partitioning
ALTER INDEX t@t_b_idx PARTITION BY LIST(d) (
PARTITION pk_implicit VALUES IN (1)
)

statement error cannot ALTER INDEX PARTITION BY on index which already has implicit column partitioning
statement error cannot ALTER INDEX PARTITION BY on an index which already has implicit column partitioning
ALTER INDEX t@t_b_idx PARTITION BY NOTHING

statement error cannot ALTER TABLE PARTITION BY on table which already has implicit column partitioning
statement error cannot ALTER TABLE PARTITION BY on a table which already has implicit column partitioning
ALTER TABLE t PARTITION BY NOTHING

statement ok
CREATE TABLE t_implicit_idx (
id INT PRIMARY KEY,
a INT,
INDEX t_a_idx (a) PARTITION BY LIST (id) (
PARTITION "one" VALUES IN (1)
)
)

statement error cannot ALTER INDEX PARTITION BY on an index which already has implicit column partitioning
ALTER INDEX t_implicit_idx@t_a_idx PARTITION BY LIST (a) (
PARTITION "one" VALUES IN (1)
)

query TTTTTT colnames
SELECT
indexrelid, indrelid, indkey, indclass, indoption, indcollation
Expand Down Expand Up @@ -360,12 +374,12 @@ t t_b_idx CREATE INDEX t_b_idx ON test.public.t USING btree (b ASC
t t_c_key CREATE UNIQUE INDEX t_c_key ON test.public.t USING btree (c ASC)
t t_j_idx CREATE INDEX t_j_idx ON test.public.t USING gin (j ASC)

statement error cannot ALTER INDEX PARTITION BY on index which already has implicit column partitioning
statement error cannot ALTER INDEX PARTITION BY on an index which already has implicit column partitioning
ALTER INDEX new_idx PARTITION BY LIST (a) (
PARTITION d_implicit VALUES IN (1)
)

statement error cannot ALTER TABLE PARTITION BY on table which already has implicit column partitioning
statement error cannot ALTER TABLE PARTITION BY on a table which already has implicit column partitioning
ALTER TABLE t PARTITION BY LIST (a) (
PARTITION pk_implicit VALUES IN (1)
)
Expand Down Expand Up @@ -431,6 +445,11 @@ CREATE INDEX created_idx ON t(c) PARTITION BY LIST (d) (
PARTITION one VALUES IN ((1))
)

statement error cannot change the partitioning of an index if the table has PARTITION ALL BY defined
ALTER INDEX t@t_a_idx PARTITION BY LIST (a) (
PARTITION one VALUES IN (1)
)

query T noticetrace
CREATE INDEX created_idx ON t(c)
----
Expand Down
58 changes: 46 additions & 12 deletions pkg/ccl/logictestccl/testdata/logic_test/regional_by_row
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,44 @@
statement ok
CREATE DATABASE multi_region_test_db PRIMARY REGION "ca-central-1" REGIONS "ap-southeast-2", "us-east-1" SURVIVE REGION FAILURE

statement error cannot set LOCALITY on a database that is not multi-region enabled
statement error cannot set LOCALITY on a table in a database that is not multi-region enabled
CREATE TABLE regional_by_row_table (pk int) LOCALITY REGIONAL BY ROW

statement ok
USE multi_region_test_db

statement error REGIONAL BY ROW on a TABLE containing PARTITION BY is not supported
statement error multi-region tables containing PARTITION BY are not supported
CREATE TABLE regional_by_row_table (
pk int
)
PARTITION BY LIST (pk) (PARTITION one VALUES IN ((1)))
LOCALITY REGIONAL BY ROW

statement error REGIONAL BY ROW on a TABLE containing PARTITION BY is not supported
statement error multi-region tables containing PARTITION BY are not supported
CREATE TABLE regional_by_row_table (
pk int
)
PARTITION BY NOTHING
LOCALITY REGIONAL BY ROW

statement error multi-region tables with an INDEX containing PARTITION BY are not supported
CREATE TABLE regional_by_row_table (
pk int,
INDEX idx(id) PARTITION BY LIST (id) (
PARTITION "pk" VALUES IN (1)
)
)
LOCALITY REGIONAL BY ROW

statement error multi-region tables with an UNIQUE constraint containing PARTITION BY are not supported
CREATE TABLE regional_by_row_table (
pk int,
UNIQUE INDEX idx(id) PARTITION BY LIST (id) (
PARTITION "pk" VALUES IN (1)
)
)
LOCALITY REGIONAL BY ROW

statement ok
SET experimental_enable_hash_sharded_indexes = true

Expand All @@ -45,23 +63,23 @@ CREATE TABLE regional_by_row_table (
INDEX(a) USING HASH WITH BUCKET_COUNT = 8
) LOCALITY REGIONAL BY ROW

statement error REGIONAL BY ROW on a table with an INDEX containing PARTITION BY is not supported
statement error multi-region tables with an INDEX containing PARTITION BY are not supported
CREATE TABLE regional_by_row_table (
pk int,
a int,
INDEX (a) PARTITION BY LIST (a) (PARTITION one VALUES IN ((1)))
)
LOCALITY REGIONAL BY ROW

statement error REGIONAL BY ROW on a table with an INDEX containing PARTITION BY is not supported
statement error multi-region tables with an INDEX containing PARTITION BY are not supported
CREATE TABLE regional_by_row_table (
pk int,
a int,
INDEX (a) PARTITION BY NOTHING
)
LOCALITY REGIONAL BY ROW

statement error REGIONAL BY ROW on a table with an INDEX containing PARTITION BY is not supported
statement error multi-region tables with an INDEX containing PARTITION BY are not supported
CREATE TABLE regional_by_row_table (
pk int,
a int,
Expand All @@ -70,15 +88,15 @@ CREATE TABLE regional_by_row_table (
)
LOCALITY REGIONAL BY ROW

statement error REGIONAL BY ROW on a table with an UNIQUE constraint containing PARTITION BY is not supported
statement error multi-region tables with an UNIQUE constraint containing PARTITION BY are not supported
CREATE TABLE regional_by_row_table (
pk int,
a int,
UNIQUE (a) 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
statement error multi-region tables with an UNIQUE constraint containing PARTITION BY are not supported
CREATE TABLE regional_by_row_table (
pk int,
a int,
Expand Down Expand Up @@ -470,6 +488,22 @@ CREATE TABLE public.regional_by_row_table (
ALTER PARTITION "us-east-1" OF INDEX multi_region_test_db.public.regional_by_row_table@regional_by_row_table_a_idx CONFIGURE ZONE USING "gc.ttlseconds" = 10

# Prohibit certain actions on a REGIONAL BY ROW table.

statement error cannot set PARTITION BY on a table in a multi-region enabled database
ALTER TABLE regional_by_row_table PARTITION BY LIST (pk) (
PARTITION "one" VALUES IN (1)
)

statement error cannot change the partitioning of an index if the table is part of a multi-region database
ALTER INDEX regional_by_row_table@regional_by_row_table_a_idx PARTITION BY LIST (pk2) (
PARTITION one VALUES IN (1)
)

statement error cannot define PARTITION BY on a new INDEX in a multi-region database
CREATE INDEX bad_idx ON regional_by_row_table(a) PARTITION BY LIST (a) (
PARTITION one VALUES IN (1)
)

statement error hash sharded indexes are not compatible with REGIONAL BY ROW tables
CREATE INDEX bad_idx ON regional_by_row_table(a) USING HASH WITH BUCKET_COUNT = 8

Expand Down Expand Up @@ -609,7 +643,7 @@ SELECT message FROM [SHOW KV TRACE FOR SESSION] WITH ORDINALITY
OR message LIKE 'Scan%'
ORDER BY ordinality ASC
----
Scan /Table/71/1/"@"/1{-/#}, /Table/71/1/"\x80"/1{-/#}, /Table/71/1/"\xc0"/1{-/#}
Scan /Table/73/1/"@"/1{-/#}, /Table/73/1/"\x80"/1{-/#}, /Table/73/1/"\xc0"/1{-/#}
fetched: /regional_by_row_table/primary/'ap-southeast-2'/1/pk2/a/b/j -> /1/2/3/'{"a": "b"}'
output row: [1 1 2 3 '{"a": "b"}']

Expand Down Expand Up @@ -648,7 +682,7 @@ SELECT message FROM [SHOW KV TRACE FOR SESSION] WITH ORDINALITY
OR message LIKE 'Scan%'
ORDER BY ordinality ASC
----
Scan /Table/71/1/"@"/1{-/#}
Scan /Table/73/1/"@"/1{-/#}
fetched: /regional_by_row_table/primary/'ap-southeast-2'/1/pk2/a/b/j -> /1/2/3/'{"a": "b"}'
output row: [1 1 2 3 '{"a": "b"}']

Expand All @@ -663,8 +697,8 @@ SELECT message FROM [SHOW KV TRACE FOR SESSION] WITH ORDINALITY
OR message LIKE 'Scan%'
ORDER BY ordinality ASC
----
Scan /Table/71/1/"@"/10{-/#}
Scan /Table/71/1/"\x80"/10{-/#}, /Table/71/1/"\xc0"/10{-/#}
Scan /Table/73/1/"@"/10{-/#}
Scan /Table/73/1/"\x80"/10{-/#}, /Table/73/1/"\xc0"/10{-/#}
fetched: /regional_by_row_table/primary/'ca-central-1'/10/pk2/a/b -> /10/11/12
output row: [10 10 11 12 NULL]

Expand Down
Loading

0 comments on commit f02e38f

Please sign in to comment.