Skip to content

Commit

Permalink
Merge #59364
Browse files Browse the repository at this point in the history 
59364: sql: handle CONSTRAINT...UNIQUE with PARTITION BY ALL / REGIONAL BY ROW r=ajstorm a=otan

sql: teach ADD UNIQUE CONSTRAINT to respect PARTITION ALL BY

Release note (sql change): Fix-up ALTER TABLE ... ADD CONSTRAINT ...
UNIQUE to partition correctly under a PARTITION ALL BY table.

sql: respect REGIONAL BY ROW when adding a new UNIQUE CONSTRAINT

Release note (sql change): This change teaches CRDB to apply zone
configs to new unique constraints in REGIONAL BY ROW tables.



Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
  • Loading branch information
@otan
craig[bot] and otan committed Jan 26, 2021
2 parents cc12bbc + 50aadd7 commit aaa44bd
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 45 deletions.
37 changes: 24 additions & 13 deletions pkg/ccl/logictestccl/testdata/logic_test/partitioning_implicit
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,10 +221,10 @@ CREATE TABLE public.t (
pk int PRIMARY KEY,
pk2 int NOT NULL,
partition_by int,
a int,
b int,
c int,
d int,
a int NOT NULL,
b int NOT NULL,
c int NOT NULL,
d int NOT NULL,
INDEX (a),
UNIQUE (b),
INDEX (partition_by, c),
Expand All @@ -242,22 +242,26 @@ CREATE INDEX created_idx ON t(c) PARTITION BY LIST (d) (
statement ok
CREATE INDEX created_idx ON t(c)

statement ok
ALTER TABLE t ADD CONSTRAINT unique_c_d UNIQUE(c, d)

query T
SELECT create_statement FROM [SHOW CREATE TABLE t]
----
CREATE TABLE public.t (
pk INT8 NOT NULL,
pk2 INT8 NOT NULL,
partition_by INT8 NULL,
a INT8 NULL,
b INT8 NULL,
c INT8 NULL,
d INT8 NULL,
a INT8 NOT NULL,
b INT8 NOT NULL,
c INT8 NOT NULL,
d INT8 NOT NULL,
CONSTRAINT "primary" PRIMARY KEY (pk ASC),
INDEX t_a_idx (a ASC),
UNIQUE INDEX t_b_key (b ASC),
INDEX t_partition_by_c_idx (partition_by ASC, c ASC),
INDEX created_idx (c ASC),
UNIQUE INDEX unique_c_d (c ASC, d ASC),
FAMILY fam_0_pk_pk2_partition_by_a_b_c_d (pk, pk2, partition_by, a, b, c, d)
) PARTITION ALL BY LIST (partition_by) (
PARTITION one VALUES IN ((1)),
Expand All @@ -281,6 +285,9 @@ t_b_key b false
t_b_key partition_by true
t_partition_by_c_idx c false
t_partition_by_c_idx partition_by false
unique_c_d c false
unique_c_d d false
unique_c_d partition_by true

statement ok
ALTER TABLE t ALTER PRIMARY KEY USING COLUMNS (pk2)
Expand All @@ -292,16 +299,17 @@ CREATE TABLE public.t (
pk INT8 NOT NULL,
pk2 INT8 NOT NULL,
partition_by INT8 NULL,
a INT8 NULL,
b INT8 NULL,
c INT8 NULL,
d INT8 NULL,
a INT8 NOT NULL,
b INT8 NOT NULL,
c INT8 NOT NULL,
d INT8 NOT NULL,
CONSTRAINT "primary" PRIMARY KEY (pk2 ASC),
UNIQUE INDEX t_pk_key (pk ASC),
INDEX t_a_idx (a ASC),
UNIQUE INDEX t_b_key (b ASC),
INDEX t_partition_by_c_idx (partition_by ASC, c ASC),
INDEX created_idx (c ASC),
UNIQUE INDEX unique_c_d (c ASC, d ASC),
FAMILY fam_0_pk_pk2_partition_by_a_b_c_d (pk, pk2, partition_by, a, b, c, d)
) PARTITION ALL BY LIST (partition_by) (
PARTITION one VALUES IN ((1)),
Expand All @@ -327,11 +335,14 @@ t_partition_by_c_idx c false
t_partition_by_c_idx partition_by false
t_pk_key partition_by true
t_pk_key pk false
unique_c_d c false
unique_c_d d false
unique_c_d partition_by true

statement ok
DROP TABLE t

# Tests for PARTITION ALL BY RANGE
# Tests for PARTITION ALL BY RANGE.
statement ok
CREATE TABLE public.t (
pk int PRIMARY KEY,
Expand Down
33 changes: 26 additions & 7 deletions pkg/ccl/logictestccl/testdata/logic_test/regional_by_row
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ LOCALITY REGIONAL BY ROW
statement ok
CREATE TABLE regional_by_row_table (
pk int PRIMARY KEY,
a int,
b int,
a int NOT NULL,
b int NOT NULL,
INDEX (a),
UNIQUE (b),
FAMILY (pk, a, b)
Expand All @@ -88,8 +88,8 @@ SELECT create_statement FROM [SHOW CREATE TABLE regional_by_row_table]
----
CREATE TABLE public.regional_by_row_table (
pk INT8 NOT NULL,
a INT8 NULL,
b INT8 NULL,
a INT8 NOT NULL,
b INT8 NOT NULL,
crdb_region public.crdb_internal_region NOT NULL DEFAULT gateway_region()::public.crdb_internal_region,
CONSTRAINT "primary" PRIMARY KEY (pk ASC),
INDEX regional_by_row_table_a_idx (a ASC),
Expand Down Expand Up @@ -199,22 +199,26 @@ ca-central-1 10 11 12
us-east-1 20 21 22
ap-southeast-2 23 24 25

# Tests creating a index on a REGIONAL BY ROW table.
# Tests creating a index and a unique constraint on a REGIONAL BY ROW table.
statement ok
CREATE INDEX new_idx ON regional_by_row_table(a, b)

statement ok
ALTER TABLE regional_by_row_table ADD CONSTRAINT unique_b_a UNIQUE(b, a)

query T
SELECT create_statement FROM [SHOW CREATE TABLE regional_by_row_table]
----
CREATE TABLE public.regional_by_row_table (
pk INT8 NOT NULL,
a INT8 NULL,
b INT8 NULL,
a INT8 NOT NULL,
b INT8 NOT NULL,
crdb_region public.crdb_internal_region 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),
INDEX new_idx (a ASC, b ASC),
UNIQUE INDEX unique_b_a (b ASC, a ASC),
FAMILY fam_0_pk_a_b_crdb_region (pk, a, b, crdb_region)
) LOCALITY REGIONAL BY ROW;
ALTER PARTITION "ap-southeast-2" OF INDEX multi_region_test_db.public.regional_by_row_table@regional_by_row_table_a_idx CONFIGURE ZONE USING
Expand Down Expand Up @@ -262,6 +266,18 @@ ALTER PARTITION "ca-central-1" OF INDEX multi_region_test_db.public.regional_by_
constraints = '{+region=ca-central-1: 1}',
lease_preferences = '[[+region=ca-central-1]]';
ALTER PARTITION "us-east-1" OF INDEX multi_region_test_db.public.regional_by_row_table@new_idx CONFIGURE ZONE USING
num_replicas = 3,
constraints = '{+region=us-east-1: 1}',
lease_preferences = '[[+region=us-east-1]]';
ALTER PARTITION "ap-southeast-2" OF INDEX multi_region_test_db.public.regional_by_row_table@unique_b_a CONFIGURE ZONE USING
num_replicas = 3,
constraints = '{+region=ap-southeast-2: 1}',
lease_preferences = '[[+region=ap-southeast-2]]';
ALTER PARTITION "ca-central-1" OF INDEX multi_region_test_db.public.regional_by_row_table@unique_b_a CONFIGURE ZONE USING
num_replicas = 3,
constraints = '{+region=ca-central-1: 1}',
lease_preferences = '[[+region=ca-central-1]]';
ALTER PARTITION "us-east-1" OF INDEX multi_region_test_db.public.regional_by_row_table@unique_b_a CONFIGURE ZONE USING
num_replicas = 3,
constraints = '{+region=us-east-1: 1}',
lease_preferences = '[[+region=us-east-1]]'
Expand All @@ -281,6 +297,9 @@ 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
unique_b_a a false
unique_b_a b false
unique_b_a crdb_region true

# Tests for REGIONAL BY TABLE AS
statement error cannot use column crdb_region_col which has type INT8 in REGIONAL BY ROW AS\nDETAIL:\s+REGIONAL BY ROW AS must reference a column of type crdb_internal_region.
Expand Down
47 changes: 22 additions & 25 deletions pkg/sql/alter_table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,31 +220,16 @@ func (n *alterTableNode) startExec(params runParams) error {
if err := idx.FillColumns(d.Columns); err != nil {
return err
}
if d.PartitionByIndex.ContainsPartitions() {
var numImplicitColumns int
var err error
idx, numImplicitColumns, err = detectImplicitPartitionColumns(
params.EvalContext(),
n.tableDesc,
idx,
d.PartitionByIndex.PartitionBy,
)
if err != nil {
return err
}
partitioning, err := CreatePartitioning(
params.ctx,
params.p.ExecCfg().Settings,
params.EvalContext(),
n.tableDesc,
&idx,
numImplicitColumns,
d.PartitionByIndex.PartitionBy,
)
if err != nil {
return err
}
idx.Partitioning = partitioning

var err error
idx, err = params.p.configureIndexDescForNewIndexPartitioning(
params.ctx,
n.tableDesc,
idx,
d.PartitionByIndex,
)
if err != nil {
return err
}
foundIndex, err := n.tableDesc.FindIndexWithName(string(d.Name))
if err == nil {
Expand All @@ -257,6 +242,18 @@ func (n *alterTableNode) startExec(params runParams) error {
return err
}

// We need to allocate IDs upfront in the event we need to update the zone config
// in the same transaction.
if err := n.tableDesc.AllocateIDs(params.ctx); err != nil {
return err
}
if err := params.p.configureZoneConfigForNewIndexPartitioning(
params.ctx,
n.tableDesc,
idx,
); err != nil {
return err
}
case *tree.CheckConstraintTableDef:
var err error
params.p.runWithOptions(resolveFlags{contextDatabaseID: n.tableDesc.ParentID}, func() {
Expand Down

0 comments on commit aaa44bd

Please sign in to comment.