Skip to content

Commit

Permalink
Merge #126531
Browse files Browse the repository at this point in the history 
126531: sql: ensure truncate preserves zone configs r=fqazi a=annrpom

Previously, we did not copy zone configs over
from old indexes to new ones created during a
truncate for pkeys. This was due to truncate
re-using the same logic as our `PrimaryKeySwap`
mutation. This patch adds an override in said logic to
allow for zcs to be copied from old to new indexes.

Epic: [CRDB-22729](https://cockroachlabs.atlassian.net/browse/CRDB-22729)
Fixes: #94188

Release note (bug fix): Fixed bug where zone configs
on pkeys disappeared during truncate.

Co-authored-by: Annie Pompa <[email protected]>
  • Loading branch information
@annrpom
craig[bot] and annrpom committed Jul 9, 2024
2 parents 9d66cfb + fc80ff3 commit 0b918d1
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 6 deletions.
83 changes: 83 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/partitioning
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1411,3 +1411,86 @@ vectorized: true
estimated row count: 555,555,557 - 378,667,879,137,377,664 (11% of the table; stats collected <hidden> ago)
table: t104434@t104434_col1_2_col1_6_col1_7_key (partial index)
spans: [ - /'BOX(0.8102585814674039 -0.6055208348537393,1.2525166924090267 0.4387570127404082)') [/'BOX(0.8102585814674039 -0.6055208348537393,1.2525166924090267 0.4387570127404082)'/'2012-05-14 07:37:50.000177+00' - /'BOX(0.8102585814674039 -0.6055208348537393,1.2525166924090267 0.4387570127404082)'/'2012-05-14 07:37:50.000177+00'] (/'BOX(0.8102585814674039 -0.6055208348537393,1.2525166924090267 0.4387570127404082)' - /'BOX(0.5800044253150916 -0.631859538843543,2.166538271521436 -0.3936412129189529)') [/'BOX(0.5800044253150916 -0.631859538843543,2.166538271521436 -0.3936412129189529)'/'2012-05-14 07:37:50.000177+00' - /'BOX(0.5800044253150916 -0.631859538843543,2.166538271521436 -0.3936412129189529)'/'2012-05-14 07:37:50.000177+00'] … (35 more)

# Regression test for #94188 in which we did not copy over the zone config in
# to the new indexes created after a truncate.
subtest zone_config

statement ok
CREATE TABLE bar(a int primary key) PARTITION BY LIST (a) (PARTITION x VALUES IN (1), PARTITION y VALUES IN (2));

statement ok
ALTER PARTITION x OF TABLE bar CONFIGURE ZONE USING gc.ttlseconds=100;

statement ok
ALTER PARTITION y OF TABLE bar CONFIGURE ZONE USING gc.ttlseconds=150;

query TT
SHOW CREATE TABLE bar;
----
bar CREATE TABLE public.bar (
a INT8 NOT NULL,
CONSTRAINT bar_pkey PRIMARY KEY (a ASC)
) PARTITION BY LIST (a) (
PARTITION x VALUES IN ((1)),
PARTITION y VALUES IN ((2))
);
ALTER PARTITION x OF INDEX test.public.bar@bar_pkey CONFIGURE ZONE USING
gc.ttlseconds = 100;
ALTER PARTITION y OF INDEX test.public.bar@bar_pkey CONFIGURE ZONE USING
gc.ttlseconds = 150


statement ok
TRUNCATE bar;

query TT
SHOW CREATE TABLE bar;
----
bar CREATE TABLE public.bar (
a INT8 NOT NULL,
CONSTRAINT bar_pkey PRIMARY KEY (a ASC)
) PARTITION BY LIST (a) (
PARTITION x VALUES IN ((1)),
PARTITION y VALUES IN ((2))
);
ALTER PARTITION x OF INDEX test.public.bar@bar_pkey CONFIGURE ZONE USING
gc.ttlseconds = 100;
ALTER PARTITION y OF INDEX test.public.bar@bar_pkey CONFIGURE ZONE USING
gc.ttlseconds = 150

query TT
SHOW ZONE CONFIGURATION FOR PARTITION x OF INDEX test.public.bar@bar_pkey;
----
PARTITION x OF INDEX test.public.bar@bar_pkey ALTER PARTITION x OF INDEX test.public.bar@bar_pkey CONFIGURE ZONE USING
range_min_bytes = 134217728,
range_max_bytes = 536870912,
gc.ttlseconds = 100,
num_replicas = 3,
constraints = '[]',
lease_preferences = '[]'

query TT
SHOW ZONE CONFIGURATION FOR PARTITION y OF INDEX test.public.bar@bar_pkey;
----
PARTITION y OF INDEX test.public.bar@bar_pkey ALTER PARTITION y OF INDEX test.public.bar@bar_pkey CONFIGURE ZONE USING
range_min_bytes = 134217728,
range_max_bytes = 536870912,
gc.ttlseconds = 150,
num_replicas = 3,
constraints = '[]',
lease_preferences = '[]'

query T rowsort
WITH subzone_spans AS (
SELECT json_array_elements(crdb_internal.pb_to_json('cockroach.config.zonepb.ZoneConfig', config) -> 'subzoneSpans') ->> 'key' AS key
FROM system.zones
WHERE id = 'bar'::REGCLASS::OID
)
SELECT crdb_internal.pretty_key(decode(key, 'base64'), 0) AS pretty_key
FROM subzone_spans;
----
/2/1
/2/2

subtest end
15 changes: 10 additions & 5 deletions pkg/sql/schema_changer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1971,14 +1971,16 @@ func (sc *SchemaChanger) done(ctx context.Context) error {

// maybeUpdateZoneConfigsForPKChange moves zone configs for any rewritten
// indexes from the old index over to the new index. Noop if run on behalf of a
// tenant.
// tenant. If forceSwap is set, we copy the zone configs for primary keys
// regardless of the table's locality.
func maybeUpdateZoneConfigsForPKChange(
ctx context.Context,
txn descs.Txn,
execCfg *ExecutorConfig,
kvTrace bool,
table *tabledesc.Mutable,
swapInfo *descpb.PrimaryKeySwap,
forceSwap bool,
) error {
zoneWithRaw, err := txn.Descriptors().GetZoneConfig(ctx, txn.KV(), table.GetID())
if err != nil {
Expand All @@ -1997,10 +1999,12 @@ func maybeUpdateZoneConfigsForPKChange(
oldIdxToNewIdx[oldID] = swapInfo.NewIndexes[i]
}

// It is safe to copy the zone config off a primary index of a REGIONAL BY ROW table.
// This is because the prefix of the PK we used to partition by will stay the same,
// so a direct copy will always work.
if table.IsLocalityRegionalByRow() {
// It is safe to copy the zone config off a primary index of a REGIONAL BY ROW
// table. This is because the prefix of the PK we used to partition by will
// stay the same, so a direct copy will always work. If forceSwap is true, we
// are performing an operation that does not need to worry about data (like
// for TRUNCATE).
if table.IsLocalityRegionalByRow() || forceSwap {
oldIdxToNewIdx[swapInfo.OldPrimaryIndexId] = swapInfo.NewPrimaryIndexId
}

Expand Down Expand Up @@ -3141,6 +3145,7 @@ func (sc *SchemaChanger) applyZoneConfigChangeForMutation(
// necessary.
return maybeUpdateZoneConfigsForPKChange(
ctx, txn, sc.execCfg, false /* kvTrace */, tableDesc, pkSwap.PrimaryKeySwapDesc(),
false, /* forceSwap */
)
}
return nil
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/truncate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,8 @@ func (p *planner) truncateTable(ctx context.Context, id descpb.ID, jobDesc strin
NewIndexes: newIndexIDs[1:],
}
if err := maybeUpdateZoneConfigsForPKChange(
ctx, p.InternalSQLTxn(), p.ExecCfg(), p.ExtendedEvalContext().Tracing.KVTracingEnabled(), tableDesc, swapInfo,
ctx, p.InternalSQLTxn(), p.ExecCfg(), p.ExtendedEvalContext().Tracing.KVTracingEnabled(),
tableDesc, swapInfo, true, /* forceSwap */
); err != nil {
return err
}
Expand Down

0 comments on commit 0b918d1

Please sign in to comment.