diff --git a/pkg/sql/alter_primary_key.go b/pkg/sql/alter_primary_key.go index ca19413d48e2..2167b94c5332 100644 --- a/pkg/sql/alter_primary_key.go +++ b/pkg/sql/alter_primary_key.go @@ -171,7 +171,7 @@ func (p *planner) AlterPrimaryKey( // If the new index is requested to be sharded, set up the index descriptor // to be sharded, and add the new shard column if it is missing. if alterPKNode.Sharded != nil { - shardCol, newColumns, newColumn, err := setupShardedIndex( + shardCol, newColumns, err := setupShardedIndex( ctx, p.EvalContext(), &p.semaCtx, @@ -186,15 +186,13 @@ func (p *planner) AlterPrimaryKey( return err } alterPKNode.Columns = newColumns - if newColumn { - if err := p.setupConstraintForShard( - ctx, - tableDesc, - shardCol, - newPrimaryIndexDesc.Sharded.ShardBuckets, - ); err != nil { - return err - } + if err := p.maybeSetupConstraintForShard( + ctx, + tableDesc, + shardCol, + newPrimaryIndexDesc.Sharded.ShardBuckets, + ); err != nil { + return err } telemetry.Inc(sqltelemetry.HashShardedIndexCounter) } diff --git a/pkg/sql/catalog/systemschema/system.go b/pkg/sql/catalog/systemschema/system.go index 5f4481c07c7c..bfedbe738624 100644 --- a/pkg/sql/catalog/systemschema/system.go +++ b/pkg/sql/catalog/systemschema/system.go @@ -518,11 +518,6 @@ CREATE TABLE system.statement_statistics ( metadata, statistics, plan - ), - CONSTRAINT check_crdb_internal_aggregated_ts_app_name_fingerprint_id_node_id_plan_hash_transaction_fingerprint_id_shard_8 CHECK ( - crdb_internal_aggregated_ts_app_name_fingerprint_id_node_id_plan_hash_transaction_fingerprint_id_shard_8 IN ( - 0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8 - ) ) ) ` @@ -554,11 +549,6 @@ CREATE TABLE system.transaction_statistics ( agg_interval, metadata, statistics - ), - CONSTRAINT check_crdb_internal_aggregated_ts_app_name_fingerprint_id_node_id_shard_8 CHECK ( - crdb_internal_aggregated_ts_app_name_fingerprint_id_node_id_shard_8 IN ( - 0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8 - ) ) ); ` diff --git a/pkg/sql/create_index.go b/pkg/sql/create_index.go index 5f74856199be..8c87630f4eeb 100644 --- a/pkg/sql/create_index.go +++ b/pkg/sql/create_index.go @@ -87,11 +87,11 @@ func (p *planner) CreateIndex(ctx context.Context, n *tree.CreateIndex) (planNod return &createIndexNode{tableDesc: tableDesc, n: n}, nil } -// setupConstraintForShard adds a check constraint ensuring that the shard +// maybeSetupConstraintForShard adds a check constraint ensuring that the shard // column's value is within [0..ShardBuckets-1]. This method is called when a // `CREATE INDEX`/`ALTER PRIMARY KEY` statement is issued for the creation of a // sharded index that *does not* re-use a pre-existing shard column. -func (p *planner) setupConstraintForShard( +func (p *planner) maybeSetupConstraintForShard( ctx context.Context, tableDesc *tabledesc.Mutable, shardCol catalog.Column, buckets int32, ) error { // Assign an ID to the newly-added shard column, which is needed for the creation @@ -104,31 +104,26 @@ func (p *planner) setupConstraintForShard( if err != nil { return err } - info, err := tableDesc.GetConstraintInfo() + ckBuilder := schemaexpr.MakeCheckConstraintBuilder(ctx, p.tableName, tableDesc, &p.semaCtx) + ckDesc, err := ckBuilder.Build(ckDef) if err != nil { return err } - inuseNames := make(map[string]struct{}, len(info)) - for k := range info { - inuseNames[k] = struct{}{} - } - - ckBuilder := schemaexpr.MakeCheckConstraintBuilder(ctx, p.tableName, tableDesc, &p.semaCtx) - ckName, err := ckBuilder.DefaultName(ckDef.Expr) + curConstraintInfos, err := tableDesc.GetConstraintInfo() if err != nil { return err } // Avoid creating duplicate check constraints. - if _, ok := inuseNames[ckName]; !ok { - ck, err := ckBuilder.Build(ckDef) - if err != nil { - return err + for _, info := range curConstraintInfos { + if info.CheckConstraint != nil && info.CheckConstraint.Expr == ckDesc.Expr { + return nil } - ck.Validity = descpb.ConstraintValidity_Validating - tableDesc.AddCheckMutation(ck, descpb.DescriptorMutation_ADD) } + + ckDesc.Validity = descpb.ConstraintValidity_Validating + tableDesc.AddCheckMutation(ckDesc, descpb.DescriptorMutation_ADD) return nil } @@ -225,7 +220,7 @@ func makeIndexDescriptor( if tableDesc.IsLocalityRegionalByRow() { return nil, hashShardedIndexesOnRegionalByRowError() } - shardCol, newColumns, newColumn, err := setupShardedIndex( + shardCol, newColumns, err := setupShardedIndex( params.ctx, params.EvalContext(), ¶ms.p.semaCtx, @@ -239,10 +234,10 @@ func makeIndexDescriptor( return nil, err } columns = newColumns - if newColumn { - if err := params.p.setupConstraintForShard(params.ctx, tableDesc, shardCol, indexDesc.Sharded.ShardBuckets); err != nil { - return nil, err - } + if err := params.p.maybeSetupConstraintForShard( + params.ctx, tableDesc, shardCol, indexDesc.Sharded.ShardBuckets, + ); err != nil { + return nil, err } } @@ -480,9 +475,10 @@ var hashShardedIndexesDisabledError = pgerror.Newf(pgcode.FeatureNotSupported, "hash sharded indexes require the experimental_enable_hash_sharded_indexes session variable") // setupShardedIndex creates a shard column for the given index descriptor. It -// returns the shard column, the new column list for the index, and a boolean -// which is true if the shard column was newly created. If the shard column is -// new, it is added to tableDesc. +// returns the shard column and the new column list for the index. If the shard +// column is new, either of the following happens: +// (1) the column is added to tableDesc if it's a new table; +// (2) a column mutation is added to tableDesc if the table is not new. func setupShardedIndex( ctx context.Context, evalCtx *tree.EvalContext, @@ -493,9 +489,9 @@ func setupShardedIndex( tableDesc *tabledesc.Mutable, indexDesc *descpb.IndexDescriptor, isNewTable bool, -) (shard catalog.Column, newColumns tree.IndexElemList, newColumn bool, err error) { +) (shard catalog.Column, newColumns tree.IndexElemList, err error) { if !shardedIndexEnabled { - return nil, nil, false, hashShardedIndexesDisabledError + return nil, nil, hashShardedIndexesDisabledError } colNames := make([]string, 0, len(columns)) @@ -504,12 +500,13 @@ func setupShardedIndex( } buckets, err := tabledesc.EvalShardBucketCount(ctx, semaCtx, evalCtx, bucketsExpr) if err != nil { - return nil, nil, false, err + return nil, nil, err } - shardCol, newColumn, err := maybeCreateAndAddShardCol(int(buckets), tableDesc, + shardCol, err := maybeCreateAndAddShardCol(int(buckets), tableDesc, colNames, isNewTable) + if err != nil { - return nil, nil, false, err + return nil, nil, err } shardIdxElem := tree.IndexElem{ Column: tree.Name(shardCol.GetName()), @@ -522,7 +519,7 @@ func setupShardedIndex( ShardBuckets: buckets, ColumnNames: colNames, } - return shardCol, newColumns, newColumn, nil + return shardCol, newColumns, nil } // maybeCreateAndAddShardCol adds a new hidden computed shard column (or its mutation) to @@ -530,10 +527,10 @@ func setupShardedIndex( // buckets. func maybeCreateAndAddShardCol( shardBuckets int, desc *tabledesc.Mutable, colNames []string, isNewTable bool, -) (col catalog.Column, created bool, err error) { +) (col catalog.Column, err error) { shardColDesc, err := makeShardColumnDesc(colNames, shardBuckets) if err != nil { - return nil, false, err + return nil, err } existingShardCol, err := desc.FindColumnWithName(tree.Name(shardColDesc.Name)) if err == nil && !existingShardCol.Dropped() { @@ -543,14 +540,14 @@ func maybeCreateAndAddShardCol( if !existingShardCol.IsHidden() { // The user managed to reverse-engineer our crazy shard column name, so // we'll return an error here rather than try to be tricky. - return nil, false, pgerror.Newf(pgcode.DuplicateColumn, + return nil, pgerror.Newf(pgcode.DuplicateColumn, "column %s already specified; can't be used for sharding", shardColDesc.Name) } - return existingShardCol, false, nil + return existingShardCol, nil } columnIsUndefined := sqlerrors.IsUndefinedColumnError(err) if err != nil && !columnIsUndefined { - return nil, false, err + return nil, err } if columnIsUndefined || existingShardCol.Dropped() { if isNewTable { @@ -558,10 +555,9 @@ func maybeCreateAndAddShardCol( } else { desc.AddColumnMutation(shardColDesc, descpb.DescriptorMutation_ADD) } - created = true } shardCol, err := desc.FindColumnWithName(tree.Name(shardColDesc.Name)) - return shardCol, created, err + return shardCol, err } func (n *createIndexNode) startExec(params runParams) error { diff --git a/pkg/sql/create_table.go b/pkg/sql/create_table.go index a8a49046d010..48393b2265b8 100644 --- a/pkg/sql/create_table.go +++ b/pkg/sql/create_table.go @@ -1485,7 +1485,7 @@ func NewTableDesc( if err != nil { return nil, err } - shardCol, _, err := maybeCreateAndAddShardCol(int(buckets), &desc, + shardCol, err := maybeCreateAndAddShardCol(int(buckets), &desc, []string{string(d.Name)}, true, /* isNewTable */ ) if err != nil { @@ -1597,7 +1597,7 @@ func NewTableDesc( if n.PartitionByTable.ContainsPartitions() { return nil, pgerror.New(pgcode.FeatureNotSupported, "sharded indexes don't support partitioning") } - shardCol, newColumns, newColumn, err := setupShardedIndex( + shardCol, newColumns, err := setupShardedIndex( ctx, evalCtx, semaCtx, @@ -1610,18 +1610,39 @@ func NewTableDesc( if err != nil { return nil, err } - if newColumn { - buckets, err := tabledesc.EvalShardBucketCount(ctx, semaCtx, evalCtx, d.Sharded.ShardBuckets) - if err != nil { - return nil, err - } - checkConstraint, err := makeShardCheckConstraintDef(int(buckets), shardCol) - if err != nil { - return nil, err + + buckets, err := tabledesc.EvalShardBucketCount(ctx, semaCtx, evalCtx, d.Sharded.ShardBuckets) + if err != nil { + return nil, err + } + checkConstraint, err := makeShardCheckConstraintDef(int(buckets), shardCol) + if err != nil { + return nil, err + } + + // If there is an equivalent check constraint from the CREATE TABLE (should + // be rare since we hide the constraint of shard column), we don't create a + // duplicate one. + ckBuilder := schemaexpr.MakeCheckConstraintBuilder(ctx, n.Table, &desc, semaCtx) + checkConstraintDesc, err := ckBuilder.Build(checkConstraint) + if err != nil { + return nil, err + } + for _, def := range n.Defs { + if inputCheckConstraint, ok := def.(*tree.CheckConstraintTableDef); ok { + inputCheckConstraintDesc, err := ckBuilder.Build(inputCheckConstraint) + if err != nil { + return nil, err + } + if checkConstraintDesc.Expr == inputCheckConstraintDesc.Expr { + return newColumns, nil + } } - n.Defs = append(n.Defs, checkConstraint) - cdd = append(cdd, nil) } + + n.Defs = append(n.Defs, checkConstraint) + cdd = append(cdd, nil) + return newColumns, nil } diff --git a/pkg/sql/logictest/testdata/logic_test/alter_primary_key b/pkg/sql/logictest/testdata/logic_test/alter_primary_key index d8a3d45049f8..60356dc30007 100644 --- a/pkg/sql/logictest/testdata/logic_test/alter_primary_key +++ b/pkg/sql/logictest/testdata/logic_test/alter_primary_key @@ -218,8 +218,7 @@ t CREATE TABLE public.t ( UNIQUE INDEX i5 (w ASC) STORING (y), INVERTED INDEX i6 (v), INDEX i7 (z ASC) USING HASH WITH BUCKET_COUNT = 4, - FAMILY fam_0_x_y_z_w_v (x, y, z, w, v), - CONSTRAINT check_crdb_internal_z_shard_4 CHECK (crdb_internal_z_shard_4 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8)) + FAMILY fam_0_x_y_z_w_v (x, y, z, w, v) ) # Test that the indexes we expect got rewritten. All but i3 should have been rewritten, @@ -368,9 +367,7 @@ t CREATE TABLE public.t ( CONSTRAINT t_pkey PRIMARY KEY (y ASC) USING HASH WITH BUCKET_COUNT = 10, UNIQUE INDEX t_x_key (x ASC), INDEX i1 (z ASC) USING HASH WITH BUCKET_COUNT = 5, - FAMILY fam_0_x_y_z (x, y, z), - CONSTRAINT check_crdb_internal_z_shard_5 CHECK (crdb_internal_z_shard_5 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8)), - CONSTRAINT check_crdb_internal_y_shard_10 CHECK (crdb_internal_y_shard_10 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8)) + FAMILY fam_0_x_y_z (x, y, z) ) query T @@ -429,8 +426,7 @@ t CREATE TABLE public.t ( CONSTRAINT t_pkey PRIMARY KEY (y ASC), UNIQUE INDEX t_x_key (x ASC) USING HASH WITH BUCKET_COUNT = 5, INDEX i (z ASC), - FAMILY fam_0_x_y_z (x, y, z), - CONSTRAINT check_crdb_internal_x_shard_5 CHECK (crdb_internal_x_shard_5 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8)) + FAMILY fam_0_x_y_z (x, y, z) ) query III @@ -556,8 +552,7 @@ t CREATE TABLE public.t ( rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), crdb_internal_x_shard_4 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(x)), 4:::INT8)) VIRTUAL, CONSTRAINT t_pkey PRIMARY KEY (x ASC) USING HASH WITH BUCKET_COUNT = 4, - FAMILY "primary" (x, rowid), - CONSTRAINT check_crdb_internal_x_shard_4 CHECK (crdb_internal_x_shard_4 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8)) + FAMILY "primary" (x, rowid) ) statement ok @@ -950,9 +945,7 @@ t CREATE TABLE public.t ( x INT8 NOT NULL, crdb_internal_x_shard_3 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(x)), 3:::INT8)) VIRTUAL, CONSTRAINT t_pkey PRIMARY KEY (x ASC) USING HASH WITH BUCKET_COUNT = 3, - FAMILY "primary" (x), - CONSTRAINT check_crdb_internal_x_shard_2 CHECK (crdb_internal_x_shard_2 IN (0:::INT8, 1:::INT8)), - CONSTRAINT check_crdb_internal_x_shard_3 CHECK (crdb_internal_x_shard_3 IN (0:::INT8, 1:::INT8, 2:::INT8)) + FAMILY "primary" (x) ) # Changes on a hash sharded index that change the columns will cause the old @@ -972,9 +965,7 @@ t CREATE TABLE public.t ( crdb_internal_y_shard_2 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(y)), 2:::INT8)) VIRTUAL, CONSTRAINT t_pkey PRIMARY KEY (y ASC) USING HASH WITH BUCKET_COUNT = 2, UNIQUE INDEX t_x_key (x ASC) USING HASH WITH BUCKET_COUNT = 2, - FAMILY fam_0_x_y (x, y), - CONSTRAINT check_crdb_internal_x_shard_2 CHECK (crdb_internal_x_shard_2 IN (0:::INT8, 1:::INT8)), - CONSTRAINT check_crdb_internal_y_shard_2 CHECK (crdb_internal_y_shard_2 IN (0:::INT8, 1:::INT8)) + FAMILY fam_0_x_y (x, y) ) # Regression for #49079. diff --git a/pkg/sql/logictest/testdata/logic_test/create_table b/pkg/sql/logictest/testdata/logic_test/create_table index be9cf56d1639..6d9201d2964f 100644 --- a/pkg/sql/logictest/testdata/logic_test/create_table +++ b/pkg/sql/logictest/testdata/logic_test/create_table @@ -369,8 +369,7 @@ like_hash CREATE TABLE public.like_hash ( rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), CONSTRAINT like_hash_base_pkey PRIMARY KEY (rowid ASC), INDEX like_hash_base_a_idx (a ASC) USING HASH WITH BUCKET_COUNT = 4, - FAMILY "primary" (a, rowid), - CONSTRAINT check_crdb_internal_a_shard_4 CHECK (crdb_internal_a_shard_4 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8)) + FAMILY "primary" (a, rowid) ) statement ok diff --git a/pkg/sql/logictest/testdata/logic_test/hash_sharded_index b/pkg/sql/logictest/testdata/logic_test/hash_sharded_index index 02ad7092853b..9c6343a80c39 100644 --- a/pkg/sql/logictest/testdata/logic_test/hash_sharded_index +++ b/pkg/sql/logictest/testdata/logic_test/hash_sharded_index @@ -12,8 +12,7 @@ sharded_primary CREATE TABLE public.sharded_primary ( crdb_internal_a_shard_10 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 10:::INT8)) VIRTUAL, a INT8 NOT NULL, CONSTRAINT sharded_primary_pkey PRIMARY KEY (a ASC) USING HASH WITH BUCKET_COUNT = 10, - FAMILY "primary" (a), - CONSTRAINT check_crdb_internal_a_shard_10 CHECK (crdb_internal_a_shard_10 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8)) + FAMILY "primary" (a) ) statement error pgcode 22023 BUCKET_COUNT must be a 32-bit integer greater than 1, got -1 @@ -49,8 +48,7 @@ sharded_primary CREATE TABLE public.sharded_primary ( a INT8 NOT NULL, crdb_internal_a_shard_10 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 10:::INT8)) VIRTUAL, CONSTRAINT "primary" PRIMARY KEY (a ASC) USING HASH WITH BUCKET_COUNT = 10, - FAMILY "primary" (a), - CONSTRAINT check_crdb_internal_a_shard_10 CHECK (crdb_internal_a_shard_10 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8)) + FAMILY "primary" (a) ) query TTT colnames @@ -107,8 +105,7 @@ specific_family CREATE TABLE public.specific_family ( CONSTRAINT specific_family_pkey PRIMARY KEY (rowid ASC), INDEX specific_family_b_idx (b ASC) USING HASH WITH BUCKET_COUNT = 10, FAMILY a_family (a, rowid), - FAMILY b_family (b), - CONSTRAINT check_crdb_internal_b_shard_10 CHECK (crdb_internal_b_shard_10 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8)) + FAMILY b_family (b) ) # Tests for secondary sharded indexes @@ -124,8 +121,7 @@ sharded_secondary CREATE TABLE public.sharded_secondary ( rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), CONSTRAINT sharded_secondary_pkey PRIMARY KEY (rowid ASC), INDEX sharded_secondary_a_idx (a ASC) USING HASH WITH BUCKET_COUNT = 4, - FAMILY "primary" (a, rowid), - CONSTRAINT check_crdb_internal_a_shard_4 CHECK (crdb_internal_a_shard_4 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8)) + FAMILY "primary" (a, rowid) ) statement ok @@ -147,8 +143,7 @@ sharded_secondary CREATE TABLE public.sharded_secondary ( rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), CONSTRAINT sharded_secondary_pkey PRIMARY KEY (rowid ASC), INDEX sharded_secondary_crdb_internal_a_shard_4_a_idx (a ASC) USING HASH WITH BUCKET_COUNT = 4, - FAMILY "primary" (a, rowid), - CONSTRAINT check_crdb_internal_a_shard_4 CHECK (crdb_internal_a_shard_4 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8)) + FAMILY "primary" (a, rowid) ) statement ok @@ -177,8 +172,7 @@ sharded_secondary CREATE TABLE public.sharded_secondary ( crdb_internal_a_shard_10 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 10:::INT8)) VIRTUAL, CONSTRAINT sharded_secondary_pkey PRIMARY KEY (rowid ASC), INDEX sharded_secondary_a_idx (a ASC) USING HASH WITH BUCKET_COUNT = 10, - FAMILY "primary" (a, rowid), - CONSTRAINT check_crdb_internal_a_shard_10 CHECK (crdb_internal_a_shard_10 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8)) + FAMILY "primary" (a, rowid) ) statement ok @@ -199,9 +193,7 @@ sharded_secondary CREATE TABLE public.sharded_secondary ( CONSTRAINT sharded_secondary_pkey PRIMARY KEY (rowid ASC), INDEX sharded_secondary_a_idx (a ASC) USING HASH WITH BUCKET_COUNT = 10, INDEX sharded_secondary_a_idx1 (a ASC) USING HASH WITH BUCKET_COUNT = 4, - FAMILY "primary" (a, rowid), - CONSTRAINT check_crdb_internal_a_shard_10 CHECK (crdb_internal_a_shard_10 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8)), - CONSTRAINT check_crdb_internal_a_shard_4 CHECK (crdb_internal_a_shard_4 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8)) + FAMILY "primary" (a, rowid) ) # Drop a sharded index and ensure that the shard column is dropped with it. @@ -217,8 +209,7 @@ sharded_secondary CREATE TABLE public.sharded_secondary ( crdb_internal_a_shard_4 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 4:::INT8)) VIRTUAL, CONSTRAINT sharded_secondary_pkey PRIMARY KEY (rowid ASC), INDEX sharded_secondary_a_idx1 (a ASC) USING HASH WITH BUCKET_COUNT = 4, - FAMILY "primary" (a, rowid), - CONSTRAINT check_crdb_internal_a_shard_4 CHECK (crdb_internal_a_shard_4 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8)) + FAMILY "primary" (a, rowid) ) statement ok @@ -279,8 +270,7 @@ sharded_secondary CREATE TABLE public.sharded_secondary ( INDEX sharded_secondary_a_idx (a ASC) USING HASH WITH BUCKET_COUNT = 10, INDEX sharded_secondary_a_idx1 (a ASC) USING HASH WITH BUCKET_COUNT = 10, INDEX sharded_secondary_a_idx2 (a ASC) USING HASH WITH BUCKET_COUNT = 10, - FAMILY "primary" (a, rowid), - CONSTRAINT check_crdb_internal_a_shard_10 CHECK (crdb_internal_a_shard_10 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8)) + FAMILY "primary" (a, rowid) ) @@ -302,9 +292,7 @@ sharded_primary CREATE TABLE public.sharded_primary ( crdb_internal_a_shard_4 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 4:::INT8)) VIRTUAL, CONSTRAINT "primary" PRIMARY KEY (a ASC) USING HASH WITH BUCKET_COUNT = 10, INDEX sharded_primary_a_idx (a ASC) USING HASH WITH BUCKET_COUNT = 4, - FAMILY "primary" (a), - CONSTRAINT check_crdb_internal_a_shard_10 CHECK (crdb_internal_a_shard_10 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8)), - CONSTRAINT check_crdb_internal_a_shard_4 CHECK (crdb_internal_a_shard_4 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8)) + FAMILY "primary" (a) ) statement ok @@ -320,8 +308,7 @@ sharded_primary CREATE TABLE public.sharded_primary ( a INT8 NOT NULL, crdb_internal_a_shard_10 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 10:::INT8)) VIRTUAL, CONSTRAINT "primary" PRIMARY KEY (a ASC) USING HASH WITH BUCKET_COUNT = 10, - FAMILY "primary" (a), - CONSTRAINT check_crdb_internal_a_shard_10 CHECK (crdb_internal_a_shard_10 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8)) + FAMILY "primary" (a) ) statement ok @@ -335,8 +322,7 @@ sharded_primary CREATE TABLE public.sharded_primary ( crdb_internal_a_shard_10 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 10:::INT8)) VIRTUAL, CONSTRAINT "primary" PRIMARY KEY (a ASC) USING HASH WITH BUCKET_COUNT = 10, INDEX sharded_primary_a_idx (a ASC) USING HASH WITH BUCKET_COUNT = 10, - FAMILY "primary" (a), - CONSTRAINT check_crdb_internal_a_shard_10 CHECK (crdb_internal_a_shard_10 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8)) + FAMILY "primary" (a) ) statement ok @@ -416,8 +402,7 @@ column_used_on_unsharded CREATE TABLE public.column_used_on_unsharded ( rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), CONSTRAINT column_used_on_unsharded_pkey PRIMARY KEY (rowid ASC), INDEX column_used_on_unsharded_crdb_internal_a_shard_10_idx (crdb_internal_a_shard_10 ASC), - FAMILY "primary" (a, rowid), - CONSTRAINT check_crdb_internal_a_shard_10 CHECK (crdb_internal_a_shard_10 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8)) + FAMILY "primary" (a, rowid) ) statement ok @@ -442,8 +427,7 @@ column_used_on_unsharded_create_table CREATE TABLE public.column_used_on_unshar rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), CONSTRAINT column_used_on_unsharded_create_table_pkey PRIMARY KEY (rowid ASC), INDEX column_used_on_unsharded_create_table_crdb_internal_a_shard_10_idx (crdb_internal_a_shard_10 ASC), - FAMILY "primary" (a, rowid), - CONSTRAINT check_crdb_internal_a_shard_10 CHECK (crdb_internal_a_shard_10 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8)) + FAMILY "primary" (a, rowid) ) statement ok @@ -499,9 +483,7 @@ weird_names CREATE TABLE public.weird_names ( "crdb_internal_'quotes' in the column's name_shard_4" INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes("'quotes' in the column's name")), 4:::INT8)) VIRTUAL, CONSTRAINT weird_names_pkey PRIMARY KEY ("I am a column with spaces" ASC) USING HASH WITH BUCKET_COUNT = 12, INDEX foo ("'quotes' in the column's name" ASC) USING HASH WITH BUCKET_COUNT = 4, - FAMILY "primary" ("I am a column with spaces", "'quotes' in the column's name"), - CONSTRAINT "check_crdb_internal_I am a column with spaces_shard_12" CHECK ("crdb_internal_I am a column with spaces_shard_12" IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8, 10:::INT8, 11:::INT8)), - CONSTRAINT "check_crdb_internal_'quotes' in the column's name_shard_4" CHECK ("crdb_internal_'quotes' in the column's name_shard_4" IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8)) + FAMILY "primary" ("I am a column with spaces", "'quotes' in the column's name") ) subtest column_does_not_exist @@ -577,9 +559,7 @@ rename_column CREATE TABLE public.rename_column ( crdb_internal_c2_shard_8 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(c2)), 8:::INT8)) VIRTUAL, CONSTRAINT rename_column_pkey PRIMARY KEY (c0 ASC, c1 ASC) USING HASH WITH BUCKET_COUNT = 8, INDEX rename_column_c2_idx (c2 ASC) USING HASH WITH BUCKET_COUNT = 8, - FAMILY "primary" (c0, c1, c2), - CONSTRAINT check_crdb_internal_c0_c1_shard_8 CHECK (crdb_internal_c0_c1_shard_8 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8)), - CONSTRAINT check_crdb_internal_c2_shard_8 CHECK (crdb_internal_c2_shard_8 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8)) + FAMILY "primary" (c0, c1, c2) ) statement ok @@ -603,9 +583,7 @@ rename_column CREATE TABLE public.rename_column ( crdb_internal_c3_shard_8 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(c3)), 8:::INT8)) VIRTUAL, CONSTRAINT rename_column_pkey PRIMARY KEY (c1 ASC, c2 ASC) USING HASH WITH BUCKET_COUNT = 8, INDEX rename_column_c2_idx (c3 ASC) USING HASH WITH BUCKET_COUNT = 8, - FAMILY "primary" (c1, c2, c3), - CONSTRAINT check_crdb_internal_c0_c1_shard_8 CHECK (crdb_internal_c1_c2_shard_8 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8)), - CONSTRAINT check_crdb_internal_c2_shard_8 CHECK (crdb_internal_c3_shard_8 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8)) + FAMILY "primary" (c1, c2, c3) ) query III @@ -628,9 +606,7 @@ rename_column CREATE TABLE public.rename_column ( crdb_internal_c2_shard_8 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(c2)), 8:::INT8)) VIRTUAL, CONSTRAINT rename_column_pkey PRIMARY KEY (c0 ASC, c1 ASC) USING HASH WITH BUCKET_COUNT = 8, INDEX rename_column_c2_idx (c2 ASC) USING HASH WITH BUCKET_COUNT = 8, - FAMILY "primary" (c0, c1, c2), - CONSTRAINT check_crdb_internal_c0_c1_shard_8 CHECK (crdb_internal_c0_c1_shard_8 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8)), - CONSTRAINT check_crdb_internal_c2_shard_8 CHECK (crdb_internal_c2_shard_8 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8)) + FAMILY "primary" (c0, c1, c2) ) query III @@ -807,3 +783,117 @@ INSERT INTO parent VALUES (1,1) statement ok INSERT INTO child VALUES (1,1) + +# Test creating tables with output of `SHOW CREATE TABLE` from table with +# hash-sharded index and make sure constraint of shard column is preserved and +# recognized by optimizer plan +subtest create_with_show_create + +statement ok +DROP TABLE IF EXISTS t + +statement ok +CREATE TABLE t ( + a INT PRIMARY KEY USING HASH WITH BUCKET_COUNT = 8 +); + +query T +explain (opt, catalog) select * from t +---- +TABLE t + ├── crdb_internal_a_shard_8 int4 not null as (mod(fnv32("crdb_internal.datums_to_bytes"(a)), 8:::INT8)) virtual [hidden] + ├── a int not null + ├── crdb_internal_mvcc_timestamp decimal [hidden] [system] + ├── tableoid oid [hidden] [system] + ├── CHECK (crdb_internal_a_shard_8 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8)) + └── PRIMARY INDEX t_pkey + ├── crdb_internal_a_shard_8 int4 not null as (mod(fnv32("crdb_internal.datums_to_bytes"(a)), 8:::INT8)) virtual [hidden] + └── a int not null + scan t + ├── check constraint expressions + │ └── crdb_internal_a_shard_8 IN (0, 1, 2, 3, 4, 5, 6, 7) + └── computed column expressions + └── crdb_internal_a_shard_8 + └── mod(fnv32(crdb_internal.datums_to_bytes(a)), 8) + +let $create_statement +SELECT create_statement FROM [SHOW CREATE TABLE t] + +statement ok +DROP TABLE t + +statement ok +$create_statement + +query T +SELECT @2 FROM [SHOW CREATE TABLE t] +---- +CREATE TABLE public.t ( + crdb_internal_a_shard_8 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 8:::INT8)) VIRTUAL, + a INT8 NOT NULL, + CONSTRAINT t_pkey PRIMARY KEY (a ASC) USING HASH WITH BUCKET_COUNT = 8, + FAMILY "primary" (a) +) + +query T +explain (opt, catalog) select * from t +---- +TABLE t + ├── crdb_internal_a_shard_8 int4 not null as (mod(fnv32(crdb_internal.datums_to_bytes(a)), 8:::INT8)) virtual [hidden] + ├── a int not null + ├── crdb_internal_mvcc_timestamp decimal [hidden] [system] + ├── tableoid oid [hidden] [system] + ├── CHECK (crdb_internal_a_shard_8 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8)) + └── PRIMARY INDEX t_pkey + ├── crdb_internal_a_shard_8 int4 not null as (mod(fnv32(crdb_internal.datums_to_bytes(a)), 8:::INT8)) virtual [hidden] + └── a int not null + scan t + ├── check constraint expressions + │ └── crdb_internal_a_shard_8 IN (0, 1, 2, 3, 4, 5, 6, 7) + └── computed column expressions + └── crdb_internal_a_shard_8 + └── mod(fnv32(crdb_internal.datums_to_bytes(a)), 8) + +# Make sure user defined constraint is used if it's equivalent to the shard +# column constraint would have been created. +statement ok +DROP TABLE t + +statement ok +CREATE TABLE public.t ( + crdb_internal_a_shard_8 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 8:::INT8)) VIRTUAL, + a INT8 NOT NULL, + CONSTRAINT t_pkey PRIMARY KEY (a ASC) USING HASH WITH BUCKET_COUNT = 8, + FAMILY "primary" (a), + CONSTRAINT check_crdb_internal_a_shard_8 CHECK (crdb_internal_a_shard_8 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8)) +) + +query T +SELECT @2 FROM [SHOW CREATE TABLE t] +---- +CREATE TABLE public.t ( + crdb_internal_a_shard_8 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 8:::INT8)) VIRTUAL, + a INT8 NOT NULL, + CONSTRAINT t_pkey PRIMARY KEY (a ASC) USING HASH WITH BUCKET_COUNT = 8, + FAMILY "primary" (a), + CONSTRAINT check_crdb_internal_a_shard_8 CHECK (crdb_internal_a_shard_8 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8)) +) + +query T +explain (opt, catalog) select * from t +---- +TABLE t + ├── crdb_internal_a_shard_8 int4 not null as (mod(fnv32(crdb_internal.datums_to_bytes(a)), 8:::INT8)) virtual [hidden] + ├── a int not null + ├── crdb_internal_mvcc_timestamp decimal [hidden] [system] + ├── tableoid oid [hidden] [system] + ├── CHECK (crdb_internal_a_shard_8 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8)) + └── PRIMARY INDEX t_pkey + ├── crdb_internal_a_shard_8 int4 not null as (mod(fnv32(crdb_internal.datums_to_bytes(a)), 8:::INT8)) virtual [hidden] + └── a int not null + scan t + ├── check constraint expressions + │ └── crdb_internal_a_shard_8 IN (0, 1, 2, 3, 4, 5, 6, 7) + └── computed column expressions + └── crdb_internal_a_shard_8 + └── mod(fnv32(crdb_internal.datums_to_bytes(a)), 8) diff --git a/pkg/sql/show_create_clauses.go b/pkg/sql/show_create_clauses.go index d24197404d25..b9d8d7782ba4 100644 --- a/pkg/sql/show_create_clauses.go +++ b/pkg/sql/show_create_clauses.go @@ -563,6 +563,9 @@ func showConstraintClause( f *tree.FmtCtx, ) error { for _, e := range desc.AllActiveAndInactiveChecks() { + if e.Hidden { + continue + } f.WriteString(",\n\t") if len(e.Name) > 0 { f.WriteString("CONSTRAINT ") diff --git a/pkg/sql/show_test.go b/pkg/sql/show_test.go index 0c37b98d6423..979b9e392d95 100644 --- a/pkg/sql/show_test.go +++ b/pkg/sql/show_test.go @@ -270,8 +270,7 @@ func TestShowCreateTable(t *testing.T) { rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), CONSTRAINT %[1]s_pkey PRIMARY KEY (rowid ASC), INDEX t12_a_idx (a ASC) USING HASH WITH BUCKET_COUNT = 8, - FAMILY "primary" (a, rowid), - CONSTRAINT check_crdb_internal_a_shard_8 CHECK (crdb_internal_a_shard_8 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8)) + FAMILY "primary" (a, rowid) )`, }, } diff --git a/pkg/sql/tests/system_table_test.go b/pkg/sql/tests/system_table_test.go index c9bf35ddd16a..d4f5da4aa083 100644 --- a/pkg/sql/tests/system_table_test.go +++ b/pkg/sql/tests/system_table_test.go @@ -191,21 +191,6 @@ func TestSystemTableLiterals(t *testing.T) { } require.NoError(t, catalog.ValidateSelf(gen)) - // TODO (Chengxiong) : remove this check after fixing #68031 - // These two system tables were created before we make shard column as - // virtual columns. We want to keep the hardcoded table descriptors to - // avoid system table migrations. However, in this test we run the `create - // table` statement and compare the result with the hardcoded descriptor, - // and there is discrepancy for sure. So we change the string statement to - // declare the shard column and constraint for it explicitly. The problem - // is that we only set `Hidden=true` when creating a shard column - // internally. User declared constraints has everything the same but with - // `Hidden=false`. So overriding the value here for now. Will remove it - // once we have better logic creating constraints. - if name == "statement_statistics" || name == "transaction_statistics" { - gen.TableDesc().Checks[0].Hidden = true - } - if test.pkg.TableDesc().Equal(gen.TableDesc()) { return }