Skip to content

Commit

Permalink
sql: block ALTER INDEX PARTITION BY for PARTITION BY ALL + multi-region
Browse files Browse the repository at this point in the history
Release justification: low-risk update to new functionality

Release note: None
  • Loading branch information
otan committed Mar 5, 2021
1 parent 1702de6 commit 4298126
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 9 deletions.
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
5 changes: 5 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/regional_by_row
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,11 @@ 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 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
18 changes: 16 additions & 2 deletions pkg/sql/alter_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
Expand Down Expand Up @@ -72,10 +74,22 @@ func (n *alterIndexNode) startExec(params runParams) error {
switch t := cmd.(type) {
case *tree.AlterIndexPartitionBy:
telemetry.Inc(sqltelemetry.SchemaChangeAlterCounterWithExtra("index", "partition_by"))
if n.tableDesc.GetPrimaryIndex().GetPartitioning().NumImplicitColumns > 0 {
if n.tableDesc.GetLocalityConfig() != nil {
return pgerror.Newf(
pgcode.FeatureNotSupported,
"cannot change the partitioning of an index if the table is part of a multi-region database",
)
}
if n.tableDesc.PartitionAllBy {
return pgerror.Newf(
pgcode.FeatureNotSupported,
"cannot change the partitioning of an index if the table has PARTITION ALL BY defined",
)
}
if n.indexDesc.Partitioning.NumImplicitColumns > 0 {
return unimplemented.New(
"ALTER INDEX PARTITION BY",
"cannot ALTER INDEX PARTITION BY on index which already has implicit column partitioning",
"cannot ALTER INDEX PARTITION BY on an index which already has implicit column partitioning",
)
}
allowImplicitPartitioning := params.p.EvalContext().SessionData.ImplicitColumnPartitioningEnabled ||
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/alter_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ func (n *alterTableNode) startExec(params runParams) error {
if oldPartitioning.NumImplicitColumns > 0 {
return unimplemented.NewWithIssue(
58731,
"cannot ALTER TABLE PARTITION BY on table which already has implicit column partitioning",
"cannot ALTER TABLE PARTITION BY on a table which already has implicit column partitioning",
)
}
newPrimaryIndex, err := CreatePartitioning(
Expand Down

0 comments on commit 4298126

Please sign in to comment.