From c2055ffc5235b28d37dbb89a1818e8f7df50f8da Mon Sep 17 00:00:00 2001 From: Oliver Tan Date: Fri, 22 Jan 2021 11:13:57 +1100 Subject: [PATCH 1/2] 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. --- .../testdata/logic_test/partitioning_implicit | 37 ++++++++++++------- pkg/sql/alter_table.go | 35 +++++------------- 2 files changed, 34 insertions(+), 38 deletions(-) diff --git a/pkg/ccl/logictestccl/testdata/logic_test/partitioning_implicit b/pkg/ccl/logictestccl/testdata/logic_test/partitioning_implicit index 31b8537e6306..ffcb2a0793d4 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/partitioning_implicit +++ b/pkg/ccl/logictestccl/testdata/logic_test/partitioning_implicit @@ -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), @@ -242,6 +242,9 @@ 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] ---- @@ -249,15 +252,16 @@ 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)), @@ -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) @@ -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)), @@ -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, diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index 9cdae7c79eef..0a520cf97dab 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -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 { From 50aadd71d6b6555e99e1935a36c925c543b014c7 Mon Sep 17 00:00:00 2001 From: Oliver Tan Date: Fri, 22 Jan 2021 11:19:16 +1100 Subject: [PATCH 2/2] 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. --- .../testdata/logic_test/regional_by_row | 33 +++++++++++++++---- pkg/sql/alter_table.go | 12 +++++++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/pkg/ccl/logictestccl/testdata/logic_test/regional_by_row b/pkg/ccl/logictestccl/testdata/logic_test/regional_by_row index 534217f5f7ce..a49d537a17a7 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/regional_by_row +++ b/pkg/ccl/logictestccl/testdata/logic_test/regional_by_row @@ -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) @@ -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), @@ -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 @@ -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]]' @@ -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. diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index 0a520cf97dab..dad72352f11d 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -242,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() {