Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

schemachanger: remove discard named range fallback #135909

Merged
merged 1 commit into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 43 additions & 22 deletions pkg/ccl/logictestccl/testdata/logic_test/zone
Original file line number Diff line number Diff line change
Expand Up @@ -1354,35 +1354,56 @@ CREATE TABLE person (
)
);

skipif config local-mixed-24.2 local-mixed-24.3
statement ok
SET use_declarative_schema_changer = unsafe_always;

statement ok
BEGIN;
SET LOCAL use_declarative_schema_changer = 'unsafe_always';
ALTER PARTITION australia OF TABLE person CONFIGURE ZONE USING gc.ttlseconds = 2;
ALTER PARTITION old_au OF TABLE person CONFIGURE ZONE USING gc.ttlseconds = 4;
ALTER PARTITION yung_au OF TABLE person CONFIGURE ZONE USING gc.ttlseconds = 5;
COMMIT;

query TT
SHOW CREATE person;
query TI colnames
WITH subzones AS (
SELECT
json_array_elements(
crdb_internal.pb_to_json('cockroach.config.zonepb.ZoneConfig', config) -> 'subzones'
) AS config
FROM system.zones
WHERE id = 'person'::REGCLASS::OID
),
subzone_configs AS (
SELECT
config ->> 'partitionName' AS name,
(config -> 'config' -> 'gc' ->> 'ttlSeconds')::INT AS ttl
FROM subzones
)
SELECT *
FROM subzone_configs
ORDER BY name;
----
person CREATE TABLE public.person (
name STRING NOT NULL,
country STRING NOT NULL,
birth_date DATE NOT NULL,
CONSTRAINT person_pkey PRIMARY KEY (country ASC, birth_date ASC, name ASC),
FAMILY fam_0_birth_date (birth_date),
FAMILY fam_1_country_name (country, name)
) PARTITION BY LIST (country) (
PARTITION australia VALUES IN (('AU'), ('NZ')) PARTITION BY RANGE (birth_date) (
PARTITION old_au VALUES FROM (MINVALUE) TO ('1995-01-01'),
PARTITION yung_au VALUES FROM ('1995-01-01') TO (MAXVALUE)
)
);
ALTER PARTITION australia OF INDEX foo.public.person@person_pkey CONFIGURE ZONE USING
gc.ttlseconds = 2;
ALTER PARTITION old_au OF INDEX foo.public.person@person_pkey CONFIGURE ZONE USING
gc.ttlseconds = 4;
ALTER PARTITION yung_au OF INDEX foo.public.person@person_pkey CONFIGURE ZONE USING
gc.ttlseconds = 5
name ttl
australia 2
old_au 4
yung_au 5

# Ensure that configuring the zone on all subpartitions of a partition erases
# the overarching partition's subzoneSpans to ensure that our keys are
# non-overlapping.
#
# N.B. We expect 2 spans allocated for each subpartition (old_au, yung_au),
# covering partition australia (instead of appending -- which would give us a
# total of 6).
query I
SELECT json_array_length(crdb_internal.pb_to_json('cockroach.config.zonepb.ZoneConfig', config) -> 'subzoneSpans')
FROM system.zones
WHERE id = 'person'::REGCLASS::OID
----
4

statement ok
RESET use_declarative_schema_changer;

subtest end
28 changes: 28 additions & 0 deletions pkg/ccl/schemachangerccl/backup_base_generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion pkg/config/zonepb/zone.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ func ResolveZoneSpecifier(
if id, ok := NamedZones[NamedZone(zs.NamedZone)]; ok {
return id, nil
}
return 0, fmt.Errorf("%q is not a built-in zone", string(zs.NamedZone))
return 0, pgerror.Newf(pgcode.InvalidName, "%q is not a built-in zone",
string(zs.NamedZone))
}

if zs.Database != "" {
Expand Down
36 changes: 36 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/zone_config
Original file line number Diff line number Diff line change
Expand Up @@ -414,3 +414,39 @@ statement ok
ALTER TABLE seq CONFIGURE ZONE USING gc.ttlseconds = 100000;

subtest end

subtest txn_zone_config

skipif config local-mixed-24.2 local-mixed-24.3
statement ok
SET use_declarative_schema_changer = unsafe_always;

skipif config 3node-tenant-default-configs
statement ok
BEGIN;
ALTER RANGE meta CONFIGURE ZONE USING num_replicas = 8;
ALTER RANGE meta CONFIGURE ZONE DISCARD;
ALTER RANGE meta CONFIGURE ZONE USING num_replicas = 7;
ALTER RANGE meta CONFIGURE ZONE USING gc.ttlseconds = 10000;
COMMIT;

skipif config 3node-tenant-default-configs
query TI colnames
WITH settings AS (
SELECT
(crdb_internal.pb_to_json('cockroach.config.zonepb.ZoneConfig', config) -> 'numReplicas') AS replicas,
((crdb_internal.pb_to_json('cockroach.config.zonepb.ZoneConfig', config) -> 'gc') ->> 'ttlSeconds')::INT AS ttl
FROM system.zones
-- 16 is the ID for the meta range
WHERE id = 16
)
SELECT *
FROM settings;
----
replicas ttl
7 10000

statement ok
RESET use_declarative_schema_changer;

subtest end
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
package scbuildstmt

import (
"fmt"

"github.com/cockroachdb/cockroach/pkg/config/zonepb"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scerrors"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
Expand Down Expand Up @@ -119,15 +120,14 @@ func astToZoneConfigObject(b BuildCtx, n *tree.SetZoneConfig) (zoneConfigObject,

// We are named range.
if zs.NamedZone != "" {
if n.Discard {
// TODO(annie): Support discard for named range.
return nil, scerrors.NotImplementedErrorf(n, "discarding a zone config on a named "+
"range is not supported in the DSC")
}
namedZone := zonepb.NamedZone(zs.NamedZone)
id, found := zonepb.NamedZones[namedZone]
if !found {
return nil, fmt.Errorf("%q is not a built-in zone", string(zs.NamedZone))
return nil, pgerror.Newf(pgcode.InvalidName, "%q is not a built-in zone",
string(zs.NamedZone))
}
if n.Discard && id == keys.RootNamespaceID {
return nil, pgerror.Newf(pgcode.CheckViolation, "cannot remove default zone")
}
return &namedRangeZoneConfigObj{rangeID: catid.DescID(id)}, nil
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/schemachanger/scexec/scmutationexec/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func (i *immediateVisitor) CreateDatabaseDescriptor(
func (i *immediateVisitor) AddDatabaseZoneConfig(
ctx context.Context, op scop.AddDatabaseZoneConfig,
) error {
i.ImmediateMutationStateUpdater.UpdateZoneConfig(op.DatabaseID, protoutil.Clone(&op.ZoneConfig).(*zonepb.ZoneConfig))
i.ImmediateMutationStateUpdater.UpdateZoneConfig(op.DatabaseID,
protoutil.Clone(&op.ZoneConfig).(*zonepb.ZoneConfig))
return nil
}
14 changes: 13 additions & 1 deletion pkg/sql/schemachanger/scexec/scmutationexec/range.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,18 @@ func (i *immediateVisitor) AddNamedRangeZoneConfig(
if !ok {
return errors.AssertionFailedf("unknown named zone: %s", op.RangeName)
}
i.ImmediateMutationStateUpdater.UpdateZoneConfig(catid.DescID(id), protoutil.Clone(&op.ZoneConfig).(*zonepb.ZoneConfig))
i.ImmediateMutationStateUpdater.UpdateZoneConfig(catid.DescID(id),
protoutil.Clone(&op.ZoneConfig).(*zonepb.ZoneConfig))
return nil
}

func (i *immediateVisitor) DiscardNamedRangeZoneConfig(
ctx context.Context, op scop.DiscardNamedRangeZoneConfig,
) error {
id, ok := zonepb.NamedZones[op.RangeName]
if !ok {
return errors.AssertionFailedf("unknown named zone: %s", op.RangeName)
}
i.ImmediateMutationStateUpdater.DeleteZoneConfig(catid.DescID(id))
return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ package scmutationexec
import (
"context"

"github.com/cockroachdb/cockroach/pkg/config/zonepb"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
)

func (i *immediateVisitor) DiscardZoneConfig(ctx context.Context, op scop.DiscardZoneConfig) error {
Expand All @@ -23,7 +25,8 @@ func (i *immediateVisitor) DiscardTableZoneConfig(
if zc.IsSubzonePlaceholder() && len(zc.Subzones) == 0 {
i.ImmediateMutationStateUpdater.DeleteZoneConfig(op.TableID)
} else {
i.ImmediateMutationStateUpdater.UpdateZoneConfig(op.TableID, zc)
i.ImmediateMutationStateUpdater.UpdateZoneConfig(op.TableID,
protoutil.Clone(&op.ZoneConfig).(*zonepb.ZoneConfig))
}
return nil
}
Expand Down
10 changes: 8 additions & 2 deletions pkg/sql/schemachanger/scop/immediate_mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -988,13 +988,19 @@ type CreateDatabaseDescriptor struct {
DatabaseID descpb.ID
}

// AddNamedRangeZoneConfig adds a zone config to a named range..
// AddNamedRangeZoneConfig adds a zone config to a named range.
type AddNamedRangeZoneConfig struct {
immediateMutationOp
RangeName zonepb.NamedZone
ZoneConfig zonepb.ZoneConfig
}

// DiscardNamedRangeZoneConfig discards a zone config from a named range.
type DiscardNamedRangeZoneConfig struct {
immediateMutationOp
RangeName zonepb.NamedZone
}

// AddDatabaseZoneConfig adds a zone config to a database.
type AddDatabaseZoneConfig struct {
immediateMutationOp
Expand All @@ -1015,7 +1021,7 @@ type DiscardZoneConfig struct {
type DiscardTableZoneConfig struct {
immediateMutationOp
TableID descpb.ID
ZoneConfig *zonepb.ZoneConfig
ZoneConfig zonepb.ZoneConfig
}

// DiscardSubzoneConfig discards the subzone config for the given descriptor ID.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,14 @@ func init() {
toAbsent(
scpb.Status_PUBLIC,
to(scpb.Status_ABSENT,
emit(func(this *scpb.NamedRangeZoneConfig) *scop.NotImplementedForPublicObjects {
return notImplementedForPublicObjects(this)
emit(func(this *scpb.NamedRangeZoneConfig) *scop.DiscardNamedRangeZoneConfig {
name, ok := zonepb.NamedZonesByID[uint32(this.RangeID)]
if !ok {
panic(errors.AssertionFailedf("unknown named zone with id: %d", this.RangeID))
}
return &scop.DiscardNamedRangeZoneConfig{
RangeName: name,
}
}),
),
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func init() {
emit(func(this *scpb.TableZoneConfig, md *opGenContext) *scop.DiscardTableZoneConfig {
return &scop.DiscardTableZoneConfig{
TableID: this.TableID,
ZoneConfig: this.ZoneConfig,
ZoneConfig: *this.ZoneConfig,
}
}),
),
Expand Down
Loading