Skip to content

Commit

Permalink
Merge #40650
Browse files Browse the repository at this point in the history
40650: sql: backward compat for ALTER PARTITION OF TABLE r=solongordon a=solongordon

When we made partition names index-scoped, we removed the ability to
specify secondary index partitions in the ALTER PARTITION ... OF TABLE
command. However I'm concerned this could break some backward
compatibility. I restored that behavior but added logic to throw an
error if there are multiple partitions with the same name.

Fixes #40425

Release note: None

Co-authored-by: Solon Gordon <solon@cockroachlabs.com>
  • Loading branch information
craig[bot] and solongordon committed Sep 11, 2019
2 parents 832b2bd + 65c5e37 commit fc9eb68
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 3 deletions.
28 changes: 25 additions & 3 deletions pkg/ccl/logictestccl/testdata/logic_test/zone
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ CREATE INDEX "my index" ON "my table" (x) PARTITION BY LIST (x) (
ALTER DATABASE "my database" CONFIGURE ZONE USING num_replicas = 1;
ALTER TABLE "my table" CONFIGURE ZONE USING num_replicas = 1;
ALTER INDEX "my table"@"my index" CONFIGURE ZONE USING num_replicas = 1;
ALTER PARTITION "my partition" OF TABLE "my table" CONFIGURE ZONE USING num_replicas = 1;
ALTER PARTITION "my partition" OF INDEX "my table"@primary CONFIGURE ZONE USING num_replicas = 1;
ALTER PARTITION "my partition" OF INDEX "my table"@"my index" CONFIGURE ZONE USING num_replicas = 1

query TTTTTT
Expand Down Expand Up @@ -601,8 +601,11 @@ CREATE INDEX x1 ON t2 (x) PARTITION BY LIST (x) (
);
CREATE INDEX x2 ON t2 (x) PARTITION BY LIST (x) (
PARTITION p1 VALUES IN (1),
PARTITION p2 VALUES IN (2)
);
PARTITION p2 VALUES IN (2),
PARTITION p3 VALUES IN (3)
)

statement ok
ALTER PARTITION p1 OF INDEX t2@* CONFIGURE ZONE USING num_replicas = 1

query TT
Expand All @@ -615,6 +618,25 @@ PARTITION p1 OF INDEX "my database".public.t2@x1 ALTER PARTITION p1 OF IND
PARTITION p1 OF INDEX "my database".public.t2@x2 ALTER PARTITION p1 OF INDEX "my database".public.t2@x2 CONFIGURE ZONE USING
num_replicas = 1

# ALTER PARTITION ... OF TABLE should only succeed if the partition name is
# unique across all indexes.
statement error pq: partition "p1" exists on multiple indexes of table "t2"
ALTER PARTITION p1 OF TABLE t2 CONFIGURE ZONE USING num_replicas = 1

statement ok
ALTER PARTITION p3 OF TABLE t2 CONFIGURE ZONE USING num_replicas = 1

query TT
SELECT * FROM [SHOW ALL ZONE CONFIGURATIONS] WHERE target LIKE '%t2@x2%'
----
PARTITION p1 OF INDEX "my database".public.t2@x2 ALTER PARTITION p1 OF INDEX "my database".public.t2@x2 CONFIGURE ZONE USING
num_replicas = 1
PARTITION p3 OF INDEX "my database".public.t2@x2 ALTER PARTITION p3 OF INDEX "my database".public.t2@x2 CONFIGURE ZONE USING
num_replicas = 1

statement error pq: partition "p4" does not exist on table "t2"
ALTER PARTITION p4 OF TABLE t2 CONFIGURE ZONE USING num_replicas = 1

# regression for #40417
statement ok
CREATE TABLE t40417 (x INT PRIMARY KEY)
Expand Down
19 changes: 19 additions & 0 deletions pkg/sql/set_zone_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,25 @@ func (n *setZoneConfigNode) startExec(params runParams) error {
return err
}

if n.zoneSpecifier.TargetsPartition() && len(n.zoneSpecifier.TableOrIndex.Index) == 0 && !n.allIndexes {
// Backward compatibility for ALTER PARTITION ... OF TABLE. Determine which
// index has the specified partition.
partitionName := string(n.zoneSpecifier.Partition)
indexes := table.FindIndexesWithPartition(partitionName)
switch len(indexes) {
case 0:
return fmt.Errorf("partition %q does not exist on table %q", partitionName, table.Name)
case 1:
n.zoneSpecifier.TableOrIndex.Index = tree.UnrestrictedName(indexes[0].Name)
default:
err := fmt.Errorf(
"partition %q exists on multiple indexes of table %q", partitionName, table.Name)
err = pgerror.WithCandidateCode(err, pgcode.InvalidParameterValue)
err = errors.WithHint(err, "try ALTER PARTITION ... OF INDEX ...")
return err
}
}

// If this is an ALTER ALL PARTITIONS statement, we need to find all indexes
// with the specified partition name and apply the zone configuration to all
// of them.
Expand Down
12 changes: 12 additions & 0 deletions pkg/sql/sqlbase/structured.go
Original file line number Diff line number Diff line change
Expand Up @@ -2097,6 +2097,18 @@ func (desc *TableDescriptor) validatePartitioningDescriptor(
return nil
}

// FindIndexesWithPartition returns all IndexDescriptors (potentially including
// the primary index) which have a partition with the given name.
func (desc *TableDescriptor) FindIndexesWithPartition(name string) []*IndexDescriptor {
var indexes []*IndexDescriptor
for _, idx := range desc.AllNonDropIndexes() {
if idx.FindPartitionByName(name) != nil {
indexes = append(indexes, idx)
}
}
return indexes
}

// validatePartitioning validates that any PartitioningDescriptors contained in
// table indexes are well-formed. See validatePartitioningDesc for details.
func (desc *TableDescriptor) validatePartitioning() error {
Expand Down

0 comments on commit fc9eb68

Please sign in to comment.