diff --git a/pkg/ccl/partitionccl/zone_test.go b/pkg/ccl/partitionccl/zone_test.go index 932f62c9188b..0db9cc8283b1 100644 --- a/pkg/ccl/partitionccl/zone_test.go +++ b/pkg/ccl/partitionccl/zone_test.go @@ -1067,9 +1067,7 @@ func TestGenerateSubzoneSpans(t *testing.T) { if err != nil { t.Fatalf("%+v", err) } - hasNewSubzones := false - spans, err := sql.GenerateSubzoneSpans( - cluster.NoSettings, keys.SystemSQLCodec, parse.tableDesc, parse.subzones, hasNewSubzones) + spans, err := sql.GenerateSubzoneSpans(keys.SystemSQLCodec, parse.tableDesc, parse.subzones) if err != nil { t.Fatalf("generating subzone spans: %+v", err) } diff --git a/pkg/kv/kvserver/reports/constraint_stats_report_test.go b/pkg/kv/kvserver/reports/constraint_stats_report_test.go index 92b58c1b2cfa..bcf453a8733a 100644 --- a/pkg/kv/kvserver/reports/constraint_stats_report_test.go +++ b/pkg/kv/kvserver/reports/constraint_stats_report_test.go @@ -1048,9 +1048,7 @@ func generateTableZone(t table, tableDesc descpb.TableDescriptor) (*zonepb.ZoneC // Fill in the SubzoneSpans. if tableZone != nil { var err error - tableZone.SubzoneSpans, err = sql.GenerateSubzoneSpans( - nil, keys.SystemSQLCodec, - tabledesc.NewBuilder(&tableDesc).BuildImmutableTable(), tableZone.Subzones, false /* hasNewSubzones */) + tableZone.SubzoneSpans, err = sql.GenerateSubzoneSpans(keys.SystemSQLCodec, tabledesc.NewBuilder(&tableDesc).BuildImmutableTable(), tableZone.Subzones) if err != nil { return nil, errors.Wrap(err, "error generating subzone spans") } diff --git a/pkg/sql/drop_index.go b/pkg/sql/drop_index.go index a23c5c6f8d13..42e9f1fe299c 100644 --- a/pkg/sql/drop_index.go +++ b/pkg/sql/drop_index.go @@ -355,13 +355,7 @@ func (p *planner) dropIndexByName( for _, s := range zone.Subzones { if s.IndexID != uint32(idx.GetID()) { - _, err = GenerateSubzoneSpans( - p.ExecCfg().Settings, - p.ExecCfg().Codec, - tableDesc, - zone.Subzones, - false, /* newSubzones */ - ) + _, err = GenerateSubzoneSpans(p.ExecCfg().Codec, tableDesc, zone.Subzones) if sqlerrors.IsCCLRequiredError(err) { return sqlerrors.NewCCLRequiredError(fmt.Errorf("schema change requires a CCL binary "+ "because table %q has at least one remaining index or partition with a zone config", diff --git a/pkg/sql/partition_utils.go b/pkg/sql/partition_utils.go index 2a54bdf8a48a..6139f01d0b7a 100644 --- a/pkg/sql/partition_utils.go +++ b/pkg/sql/partition_utils.go @@ -8,11 +8,9 @@ package sql import ( "bytes" - "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/config/zonepb" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" - "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/covering" @@ -64,20 +62,8 @@ import ( // TODO(benesch): remove the hasNewSubzones parameter when a statement to clear // all subzones at once is introduced. func GenerateSubzoneSpans( - st *cluster.Settings, - codec keys.SQLCodec, - tableDesc catalog.TableDescriptor, - subzones []zonepb.Subzone, - hasNewSubzones bool, + codec keys.SQLCodec, tableDesc catalog.TableDescriptor, subzones []zonepb.Subzone, ) ([]zonepb.SubzoneSpan, error) { - // Removing zone configs does not require a valid license. - if hasNewSubzones { - if err := base.CheckEnterpriseEnabled(st, - "replication zones on indexes or partitions"); err != nil { - return nil, err - } - } - // We already completely avoid creating subzone spans for dropped indexes. // Whether this was intentional is a different story, but it turns out to be // pretty sane. Dropped elements may refer to dropped types and we aren't diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/BUILD.bazel b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/BUILD.bazel index dbde7e20aa28..70f27c9daaf0 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/BUILD.bazel +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/BUILD.bazel @@ -44,7 +44,6 @@ go_library( importpath = "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scbuild/internal/scbuildstmt", visibility = ["//pkg/sql/schemachanger/scbuild:__subpackages__"], deps = [ - "//pkg/base", "//pkg/build", "//pkg/clusterversion", "//pkg/config/zonepb", diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go index 45f505be8154..9fe6ee8e10e1 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go @@ -10,7 +10,6 @@ import ( "context" "fmt" - "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/config/zonepb" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" @@ -652,10 +651,6 @@ func accumulateNewUniqueConstraints(currentZone, newZone *zonepb.ZoneConfig) []z func generateSubzoneSpans( b BuildCtx, tableID catid.DescID, subzones []zonepb.Subzone, ) ([]zonepb.SubzoneSpan, error) { - if err := base.CheckEnterpriseEnabled(b.ClusterSettings(), - "replication zones on indexes or partitions"); err != nil { - return nil, err - } // We already completely avoid creating subzone spans for dropped indexes. // Whether this was intentional is a different story, but it turns out to be // pretty sane. Dropped elements may refer to dropped types and we aren't diff --git a/pkg/sql/set_zone_config.go b/pkg/sql/set_zone_config.go index ddba430e8b19..634911d4086f 100644 --- a/pkg/sql/set_zone_config.go +++ b/pkg/sql/set_zone_config.go @@ -1009,9 +1009,7 @@ func prepareZoneConfigWrites( hasNewSubzones bool, ) (_ *zoneConfigUpdate, err error) { if len(z.Subzones) > 0 { - st := execCfg.Settings - z.SubzoneSpans, err = GenerateSubzoneSpans( - st, execCfg.Codec, table, z.Subzones, hasNewSubzones) + z.SubzoneSpans, err = GenerateSubzoneSpans(execCfg.Codec, table, z.Subzones) if err != nil { return nil, err } diff --git a/pkg/sql/sqlnoccltest/BUILD.bazel b/pkg/sql/sqlnoccltest/BUILD.bazel index ea6aaa5a28cb..14f0059b5253 100644 --- a/pkg/sql/sqlnoccltest/BUILD.bazel +++ b/pkg/sql/sqlnoccltest/BUILD.bazel @@ -2,25 +2,11 @@ load("@io_bazel_rules_go//go:def.bzl", "go_test") go_test( name = "sqlnoccltest_test", - srcs = [ - "main_test.go", - "partition_test.go", - ], + srcs = ["main_test.go"], deps = [ - "//pkg/base", - "//pkg/config/zonepb", "//pkg/security/securityassets", "//pkg/security/securitytest", "//pkg/server", - "//pkg/sql/catalog/catalogkeys", - "//pkg/sql/catalog/catpb", - "//pkg/sql/catalog/desctestutils", - "//pkg/sql/tests", "//pkg/testutils/serverutils", - "//pkg/testutils/sqlutils", - "//pkg/util/encoding", - "//pkg/util/leaktest", - "//pkg/util/log", - "//pkg/util/protoutil", ], ) diff --git a/pkg/sql/sqlnoccltest/partition_test.go b/pkg/sql/sqlnoccltest/partition_test.go deleted file mode 100644 index d518a95d7e9c..000000000000 --- a/pkg/sql/sqlnoccltest/partition_test.go +++ /dev/null @@ -1,153 +0,0 @@ -// Copyright 2018 The Cockroach Authors. -// -// Use of this software is governed by the CockroachDB Software License -// included in the /LICENSE file. - -package sqlnoccltest_test - -import ( - "context" - "testing" - - "github.com/cockroachdb/cockroach/pkg/base" - "github.com/cockroachdb/cockroach/pkg/config/zonepb" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/desctestutils" - "github.com/cockroachdb/cockroach/pkg/sql/tests" - "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" - "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" - "github.com/cockroachdb/cockroach/pkg/util/encoding" - "github.com/cockroachdb/cockroach/pkg/util/leaktest" - "github.com/cockroachdb/cockroach/pkg/util/log" - "github.com/cockroachdb/cockroach/pkg/util/protoutil" -) - -// Test the behavior of a binary that doesn't link in CCL when it comes to -// dealing with partitions. Some things are expected to work, others aren't. -func TestRemovePartitioningOSS(t *testing.T) { - defer leaktest.AfterTest(t)() - defer log.Scope(t).Close(t) - - ctx := context.Background() - srv, sqlDBRaw, kvDB := serverutils.StartServer(t, base.TestServerArgs{}) - defer srv.Stopper().Stop(ctx) - s := srv.ApplicationLayer() - sqlDB := sqlutils.MakeSQLRunner(sqlDBRaw) - - const numRows = 100 - if err := tests.CreateKVTable(sqlDBRaw, "kv", numRows); err != nil { - t.Fatal(err) - } - tableDesc := desctestutils.TestingGetMutableExistingTableDescriptor(kvDB, s.Codec(), "t", "kv") - tableKey := catalogkeys.MakeDescMetadataKey(s.Codec(), tableDesc.ID) - - // Hack in partitions. Doing this properly requires a CCL binary. - { - primaryIndex := *tableDesc.GetPrimaryIndex().IndexDesc() - primaryIndex.Partitioning = catpb.PartitioningDescriptor{ - NumColumns: 1, - Range: []catpb.PartitioningDescriptor_Range{{ - Name: "p1", - FromInclusive: encoding.EncodeIntValue(nil /* appendTo */, encoding.NoColumnID, 1), - ToExclusive: encoding.EncodeIntValue(nil /* appendTo */, encoding.NoColumnID, 2), - }}, - } - tableDesc.SetPrimaryIndex(primaryIndex) - } - - { - secondaryIndex := *tableDesc.PublicNonPrimaryIndexes()[0].IndexDesc() - secondaryIndex.Partitioning = catpb.PartitioningDescriptor{ - NumColumns: 1, - Range: []catpb.PartitioningDescriptor_Range{{ - Name: "p2", - FromInclusive: encoding.EncodeIntValue(nil /* appendTo */, encoding.NoColumnID, 1), - ToExclusive: encoding.EncodeIntValue(nil /* appendTo */, encoding.NoColumnID, 2), - }}, - } - tableDesc.SetPublicNonPrimaryIndex(1, secondaryIndex) - } - // Note that this is really a gross hack - it breaks planner caches, which - // assume that nothing is going to change out from under them like this. We - // "fix" the issue by altering the table's name to refresh the cache, below. - if err := kvDB.Put(ctx, tableKey, tableDesc.DescriptorProto()); err != nil { - t.Fatal(err) - } - sqlDB.Exec(t, "ALTER TABLE t.kv RENAME to t.kv2") - sqlDB.Exec(t, "ALTER TABLE t.kv2 RENAME to t.kv") - exp := `CREATE TABLE public.kv ( - k INT8 NOT NULL, - v INT8 NULL, - CONSTRAINT kv_pkey PRIMARY KEY (k ASC), - INDEX foo (v ASC) PARTITION BY RANGE (v) ( - PARTITION p2 VALUES FROM (1) TO (2) - ), - FAMILY fam_0_k (k), - FAMILY fam_1_v (v) -) PARTITION BY RANGE (k) ( - PARTITION p1 VALUES FROM (1) TO (2) -) --- Warning: Partitioned table with no zone configurations. -` - if a := sqlDB.QueryStr(t, "SHOW CREATE t.kv")[0][1]; exp != a { - t.Fatalf("expected:\n%s\n\ngot:\n%s\n\n", exp, a) - } - - // Hack in partition zone configs. This also requires a CCL binary to do - // properly. - zoneConfig := zonepb.ZoneConfig{ - Subzones: []zonepb.Subzone{ - { - IndexID: uint32(tableDesc.GetPrimaryIndexID()), - PartitionName: "p1", - Config: s.DefaultZoneConfig(), - }, - { - IndexID: uint32(tableDesc.PublicNonPrimaryIndexes()[0].GetID()), - PartitionName: "p2", - Config: s.DefaultZoneConfig(), - }, - }, - } - zoneConfigBytes, err := protoutil.Marshal(&zoneConfig) - if err != nil { - t.Fatal(err) - } - sqlDB.Exec(t, `INSERT INTO system.zones VALUES ($1, $2)`, tableDesc.ID, zoneConfigBytes) - for _, p := range []string{ - "PARTITION p1 OF INDEX t.public.kv@kv_pkey", - "PARTITION p2 OF INDEX t.public.kv@foo", - } { - if exists := sqlutils.ZoneConfigExists(t, sqlDB, p); !exists { - t.Fatalf("zone config for %s does not exist", p) - } - } - - // Some things don't work. - sqlDB.ExpectErr(t, - "OSS binaries do not include enterprise features", - `ALTER PARTITION p1 OF TABLE t.kv CONFIGURE ZONE USING DEFAULT`) - sqlDB.ExpectErr(t, - "OSS binaries do not include enterprise features", - `ALTER PARTITION p2 OF INDEX t.kv@foo CONFIGURE ZONE USING DEFAULT`) - - // But removing partitioning works. - sqlDB.Exec(t, `ALTER TABLE t.kv PARTITION BY NOTHING`) - sqlDB.Exec(t, `ALTER INDEX t.kv@foo PARTITION BY NOTHING`) - sqlDB.Exec(t, `DELETE FROM system.zones WHERE id = $1`, tableDesc.ID) - sqlDB.Exec(t, `ALTER TABLE t.kv PARTITION BY NOTHING`) - sqlDB.Exec(t, `ALTER INDEX t.kv@foo PARTITION BY NOTHING`) - - exp = `CREATE TABLE public.kv ( - k INT8 NOT NULL, - v INT8 NULL, - CONSTRAINT kv_pkey PRIMARY KEY (k ASC), - INDEX foo (v ASC), - FAMILY fam_0_k (k), - FAMILY fam_1_v (v) -)` - if a := sqlDB.QueryStr(t, "SHOW CREATE t.kv")[0][1]; exp != a { - t.Fatalf("expected:\n%s\n\ngot:\n%s\n\n", exp, a) - } -}