Commit 793ac986 authored by craig[bot]'s avatar craig[bot]

Merge #48510

48510: sql: preserve zone configurations after a primary key change r=lucy-zhang a=rohany

Fixes #48254.

Release note (bug fix): This PR fixes a bug where changing the primary
key of a table that had partitioned indexes could cause indexes to
lose their zone configurations. In particular, the indexes that got
rebuilt as part of a primary key change would keep their partitions
but lose the zone configurations attached to those partitions.
Co-authored-by: default avatarRohan Yadav <[email protected]>
parents c40343e7 24a78bc7
......@@ -1027,3 +1027,60 @@ WHERE table_name = 'parent_modify' AND index_name IS NULL
----
parent_modify NULL ALTER TABLE test.public.parent_modify CONFIGURE ZONE USING
gc.ttlseconds = 700
# Regression for #48254. Ensure that index rewrites in a primary key
# change don't drop zone configs on rewritten indexes.
statement ok
DROP TABLE IF EXISTS t;
CREATE TABLE t (
x INT PRIMARY KEY,
y INT NOT NULL,
z INT,
w INT,
INDEX i1 (z),
INDEX i2 (w),
FAMILY (x, y, z, w)
);
ALTER INDEX [email protected] PARTITION BY LIST (z) (
PARTITION p1 VALUES IN (1, 2),
PARTITION p2 VALUES IN (3, 4)
);
ALTER INDEX [email protected] PARTITION BY LIST (w) (
PARTITION p3 VALUES IN (5, 6),
PARTITION p4 VALUES IN (7, 8)
);
ALTER PARTITION p1 OF INDEX [email protected] CONFIGURE ZONE USING gc.ttlseconds = 15210;
ALTER PARTITION p2 OF INDEX [email protected] CONFIGURE ZONE USING gc.ttlseconds = 15213;
ALTER PARTITION p3 OF INDEX [email protected] CONFIGURE ZONE USING gc.ttlseconds = 15411;
ALTER PARTITION p4 OF INDEX [email protected] CONFIGURE ZONE USING gc.ttlseconds = 15418;
ALTER TABLE t ALTER PRIMARY KEY USING COLUMNS (y)
# Ensure that all the partitions of i1 and i2 still have their zone configs.
query TT
SHOW CREATE t
----
t CREATE TABLE t (
x INT8 NOT NULL,
y INT8 NOT NULL,
z INT8 NULL,
w INT8 NULL,
CONSTRAINT "primary" PRIMARY KEY (y ASC),
UNIQUE INDEX t_x_key (x ASC),
INDEX i1 (z ASC) PARTITION BY LIST (z) (
PARTITION p1 VALUES IN ((1), (2)),
PARTITION p2 VALUES IN ((3), (4))
),
INDEX i2 (w ASC) PARTITION BY LIST (w) (
PARTITION p3 VALUES IN ((5), (6)),
PARTITION p4 VALUES IN ((7), (8))
),
FAMILY fam_0_x_y_z_w (x, y, z, w)
);
ALTER PARTITION p1 OF INDEX [email protected] CONFIGURE ZONE USING
gc.ttlseconds = 15210;
ALTER PARTITION p2 OF INDEX [email protected] CONFIGURE ZONE USING
gc.ttlseconds = 15213;
ALTER PARTITION p3 OF INDEX [email protected] CONFIGURE ZONE USING
gc.ttlseconds = 15411;
ALTER PARTITION p4 OF INDEX [email protected] CONFIGURE ZONE USING
gc.ttlseconds = 15418
......@@ -21,6 +21,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/ccl/importccl"
"github.com/cockroachdb/cockroach/pkg/ccl/utilccl"
"github.com/cockroachdb/cockroach/pkg/config"
"github.com/cockroachdb/cockroach/pkg/config/zonepb"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
......@@ -31,6 +32,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/parser"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
"github.com/cockroachdb/cockroach/pkg/sql/tests"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
......@@ -1366,6 +1368,80 @@ func TestRepartitioning(t *testing.T) {
}
}
func TestPrimaryKeyChangeZoneConfigs(t *testing.T) {
defer leaktest.AfterTest(t)()
ctx := context.Background()
params, _ := tests.CreateTestServerParams()
s, sqlDB, kvDB := serverutils.StartServer(t, params)
defer s.Stopper().Stop(ctx)
// Write a table with some partitions into the database,
// and change its primary key.
if _, err := sqlDB.Exec(`
CREATE DATABASE t;
USE t;
CREATE TABLE t (
x INT PRIMARY KEY,
y INT NOT NULL,
z INT,
w INT,
INDEX i1 (z),
INDEX i2 (w),
FAMILY (x, y, z, w)
);
ALTER INDEX [email protected] PARTITION BY LIST (z) (
PARTITION p1 VALUES IN (1)
);
ALTER INDEX [email protected] PARTITION BY LIST (w) (
PARTITION p2 VALUES IN (3)
);
ALTER PARTITION p1 OF INDEX [email protected] CONFIGURE ZONE USING gc.ttlseconds = 15210;
ALTER PARTITION p2 OF INDEX [email protected] CONFIGURE ZONE USING gc.ttlseconds = 15213;
ALTER TABLE t ALTER PRIMARY KEY USING COLUMNS (y)
`); err != nil {
t.Fatal(err)
}
// Get the zone config corresponding to the table.
table := sqlbase.GetTableDescriptor(kvDB, keys.SystemSQLCodec, "t", "t")
kv, err := kvDB.Get(ctx, config.MakeZoneKey(uint32(table.ID)))
if err != nil {
t.Fatal(err)
}
var zone zonepb.ZoneConfig
if err := kv.ValueProto(&zone); err != nil {
t.Fatal(err)
}
// Our subzones should be spans prefixed with dropped copy of i1,
// dropped copy of i2, new copy of i1, and new copy of i2.
// These have ID's 2, 3, 6 and 7 respectively.
expectedSpans := []roachpb.Key{
table.IndexSpan(keys.SystemSQLCodec, 2 /* indexID */).Key,
table.IndexSpan(keys.SystemSQLCodec, 3 /* indexID */).Key,
table.IndexSpan(keys.SystemSQLCodec, 6 /* indexID */).Key,
table.IndexSpan(keys.SystemSQLCodec, 7 /* indexID */).Key,
}
if len(zone.SubzoneSpans) != len(expectedSpans) {
t.Fatalf("expected subzones to have length %d", len(expectedSpans))
}
// Subzone spans have the table prefix omitted.
prefix := keys.SystemSQLCodec.TablePrefix(uint32(table.ID))
for i := range expectedSpans {
// Subzone spans have the table prefix omitted.
expected := bytes.TrimPrefix(expectedSpans[i], prefix)
if !bytes.HasPrefix(zone.SubzoneSpans[i].Key, expected) {
t.Fatalf(
"expected span to have prefix %s but found %s",
expected,
zone.SubzoneSpans[i].Key,
)
}
}
}
func TestRemovePartitioningExpiredLicense(t *testing.T) {
defer leaktest.AfterTest(t)()
defer utilccl.TestingEnableEnterprise()()
......
......@@ -846,9 +846,25 @@ func (sc *SchemaChanger) done(ctx context.Context) (*sqlbase.ImmutableTableDescr
}
backrefTable.InboundFKs = append(backrefTable.InboundFKs, constraint.ForeignKey)
}
// Some primary key change specific operations need to happen before
// and after the index swap occurs.
if pkSwap := mutation.GetPrimaryKeySwap(); pkSwap != nil {
// We might have to update some zone configs for indexes that are
// being rewritten. It is important that this is done _before_ the
// index swap occurs. The logic that generates spans for subzone
// configurations removes spans for indexes in the dropping state,
// which we don't want. So, set up the zone configs before we swap.
if err := sc.maybeUpdateZoneConfigsForPKChange(
ctx, txn, sc.execCfg, scDesc.TableDesc(), pkSwap); err != nil {
return err
}
}
if err := scDesc.MakeMutationComplete(mutation); err != nil {
return err
}
if pkSwap := mutation.GetPrimaryKeySwap(); pkSwap != nil {
if fn := sc.testingKnobs.RunBeforePrimaryKeySwap; fn != nil {
fn()
......@@ -984,6 +1000,50 @@ func (sc *SchemaChanger) done(ctx context.Context) (*sqlbase.ImmutableTableDescr
return descs[sc.tableID], nil
}
// maybeUpdateZoneConfigsForPKChange moves zone configs for any rewritten
// indexes from the old index over to the new index.
func (sc *SchemaChanger) maybeUpdateZoneConfigsForPKChange(
ctx context.Context,
txn *kv.Txn,
execCfg *ExecutorConfig,
table *sqlbase.TableDescriptor,
swapInfo *sqlbase.PrimaryKeySwap,
) error {
zone, err := getZoneConfigRaw(ctx, txn, table.ID)
if err != nil {
return err
}
// If this table doesn't have a zone attached to it, don't do anything.
if zone == nil {
return nil
}
// For each rewritten index, point its subzones for the old index at the
// new index.
for i, oldID := range swapInfo.OldIndexes {
for j := range zone.Subzones {
subzone := &zone.Subzones[j]
if subzone.IndexID == uint32(oldID) {
// If we find a subzone matching an old index, copy its subzone
// into a new subzone with the new index's ID.
subzoneCopy := *subzone
subzoneCopy.IndexID = uint32(swapInfo.NewIndexes[i])
zone.SetSubzone(subzoneCopy)
}
}
}
// Write the zone back. This call regenerates the index spans that apply
// to each partition in the index.
_, err = writeZoneConfig(ctx, txn, table.ID, table, zone, execCfg, false)
if err != nil && !sqlbase.IsCCLRequiredError(err) {
return err
}
return nil
}
// notFirstInLine returns true whenever the schema change has been queued
// up for execution after another schema change.
func (sc *SchemaChanger) notFirstInLine(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment