From 19ce4ec609dfc999b75dc561ea799e6199e0b897 Mon Sep 17 00:00:00 2001 From: Jason Chan Date: Thu, 9 Jun 2022 09:12:51 -0700 Subject: [PATCH] sql: fix CREATE TABLE LIKE with implicit pk Previously, `CREATE TABLE LIKE` copied implicitly created columns (e.g. for the rowid default primary key and hash sharded index). Defaults for some of these columns were not properly copied over in some cases, causing unexpected constraint violations to surface. This commit fixes this by skipping copying such columns; instead, they will be freshly created. Followup work is needed for REGIONAL BY ROW. Fixes #82401 Release note: None --- pkg/sql/create_table.go | 47 ++++++++++++++----- .../testdata/logic_test/create_table | 13 +++-- .../logic_test/inverted_index_multi_column | 2 +- .../testdata/logic_test/partial_index | 2 +- .../testdata/logic_test/virtual_columns | 2 +- 5 files changed, 43 insertions(+), 23 deletions(-) diff --git a/pkg/sql/create_table.go b/pkg/sql/create_table.go index ad4169d6cf3f..c8ea7c1b30e4 100644 --- a/pkg/sql/create_table.go +++ b/pkg/sql/create_table.go @@ -2512,18 +2512,11 @@ func replaceLikeTableOpts(n *tree.CreateTable, params runParams) (tree.TableDefs // This is required to ensure the newly created table still works as expected // as these columns are required for certain features to work when used // as an index. + // TODO(#82672): We shouldn't need this. This is only still required for + // the REGIONAL BY ROW column. shouldCopyColumnDefaultSet := make(map[string]struct{}) if opts.Has(tree.LikeTableOptIndexes) { for _, idx := range td.NonDropIndexes() { - // Copy the rowid default if it was created implicitly by not specifying - // PRIMARY KEY. - if idx.Primary() && td.IsPrimaryIndexDefaultRowID() { - for i := 0; i < idx.NumKeyColumns(); i++ { - shouldCopyColumnDefaultSet[idx.GetKeyColumnName(i)] = struct{}{} - } - } - // Copy any implicitly created columns (e.g. hash-sharded indexes, - // REGIONAL BY ROW). for i := 0; i < idx.ExplicitColumnStartIdx(); i++ { for i := 0; i < idx.NumKeyColumns(); i++ { shouldCopyColumnDefaultSet[idx.GetKeyColumnName(i)] = struct{}{} @@ -2533,12 +2526,15 @@ func replaceLikeTableOpts(n *tree.CreateTable, params runParams) (tree.TableDefs } defs := make(tree.TableDefs, 0) - // Add all columns. Columns are always added. + // Add user-defined columns. for i := range td.Columns { c := &td.Columns[i] - if c.Inaccessible { - // Inaccessible columns automatically get added by - // the system; we don't need to add them ourselves here. + implicit, err := isImplicitlyCreatedBySystem(td, c) + if err != nil { + return nil, err + } + if implicit { + // Don't add system-created implicit columns. continue } def := tree.ColumnTableDef{ @@ -2618,6 +2614,11 @@ func replaceLikeTableOpts(n *tree.CreateTable, params runParams) (tree.TableDefs } if opts.Has(tree.LikeTableOptIndexes) { for _, idx := range td.NonDropIndexes() { + if idx.Primary() && td.IsPrimaryIndexDefaultRowID() { + // We won't copy over the default rowid primary index; instead + // we'll just generate a new one. + continue + } indexDef := tree.IndexTableDef{ Name: tree.Name(idx.GetName()), Inverted: idx.GetType() == descpb.IndexDescriptor_INVERTED, @@ -2886,3 +2887,23 @@ func validateUniqueConstraintParamsForCreateTableAs(n *tree.CreateTable) error { } return nil } + +// Checks if the column was automatically added by the system (e.g. for a rowid +// primary key or hash sharded index). +func isImplicitlyCreatedBySystem(td *tabledesc.Mutable, c *descpb.ColumnDescriptor) (bool, error) { + // TODO(#82672): add check for REGIONAL BY ROW column + if td.IsPrimaryIndexDefaultRowID() && c.ID == td.GetPrimaryIndex().GetKeyColumnID(0) { + return true, nil + } + col, err := td.FindColumnWithID(c.ID) + if err != nil { + return false, err + } + if td.IsShardColumn(col) { + return true, nil + } + if c.Inaccessible { + return true, nil + } + return false, nil +} diff --git a/pkg/sql/logictest/testdata/logic_test/create_table b/pkg/sql/logictest/testdata/logic_test/create_table index 72cb15ecdb50..1607f7b83cd2 100644 --- a/pkg/sql/logictest/testdata/logic_test/create_table +++ b/pkg/sql/logictest/testdata/logic_test/create_table @@ -296,7 +296,7 @@ like_no_pk_rowid_hidden CREATE TABLE public.like_no_pk_rowid_hidden ( a INT8 NULL, b INT8 NULL, rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), - CONSTRAINT like_no_pk_table_pkey PRIMARY KEY (rowid ASC) + CONSTRAINT like_no_pk_rowid_hidden_pkey PRIMARY KEY (rowid ASC) ) statement error duplicate column name @@ -323,9 +323,8 @@ like_more_specifiers CREATE TABLE public.like_more_specifiers ( t TIMESTAMPTZ NULL, z DECIMAL NULL, blah INT8 NULL, - rowid INT8 NOT VISIBLE NOT NULL, - rowid_1 INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), - CONSTRAINT like_more_specifiers_pkey PRIMARY KEY (rowid_1 ASC), + rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), + CONSTRAINT like_more_specifiers_pkey PRIMARY KEY (rowid ASC), INDEX like_more_specifiers_a_blah_z_idx (a ASC, blah ASC, z ASC) ) @@ -340,9 +339,9 @@ SHOW CREATE TABLE like_hash ---- like_hash CREATE TABLE public.like_hash ( a INT8 NULL, - crdb_internal_a_shard_4 INT8 NOT VISIBLE NOT NULL, + crdb_internal_a_shard_4 INT8 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 4:::INT8)) VIRTUAL, rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), - CONSTRAINT like_hash_base_pkey PRIMARY KEY (rowid ASC), + CONSTRAINT like_hash_pkey PRIMARY KEY (rowid ASC), INDEX like_hash_base_a_idx (a ASC) USING HASH WITH (bucket_count=4) ) @@ -359,7 +358,7 @@ like_hash CREATE TABLE public.like_hash ( a INT8 NULL, crdb_internal_a_shard_4 INT8 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 4:::INT8)) VIRTUAL, rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), - CONSTRAINT like_hash_base_pkey PRIMARY KEY (rowid ASC), + CONSTRAINT like_hash_pkey PRIMARY KEY (rowid ASC), INDEX like_hash_base_a_idx (a ASC) USING HASH WITH (bucket_count=4) ) diff --git a/pkg/sql/logictest/testdata/logic_test/inverted_index_multi_column b/pkg/sql/logictest/testdata/logic_test/inverted_index_multi_column index 2d9d4e478ae2..70c68f0d78c5 100644 --- a/pkg/sql/logictest/testdata/logic_test/inverted_index_multi_column +++ b/pkg/sql/logictest/testdata/logic_test/inverted_index_multi_column @@ -103,7 +103,7 @@ CREATE TABLE public.dst ( b INT8 NULL, j JSONB NULL, rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), - CONSTRAINT src_pkey PRIMARY KEY (rowid ASC), + CONSTRAINT dst_pkey PRIMARY KEY (rowid ASC), INVERTED INDEX src_a_j_idx (a ASC, j ASC), INVERTED INDEX src_a_b_j_idx (a ASC, b ASC, j ASC) ) diff --git a/pkg/sql/logictest/testdata/logic_test/partial_index b/pkg/sql/logictest/testdata/logic_test/partial_index index 0d0f9752d852..38ac5f1ba309 100644 --- a/pkg/sql/logictest/testdata/logic_test/partial_index +++ b/pkg/sql/logictest/testdata/logic_test/partial_index @@ -217,7 +217,7 @@ t10 CREATE TABLE public.t10 ( a INT8 NULL, b INT8 NULL, rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), - CONSTRAINT t9_pkey PRIMARY KEY (rowid ASC), + CONSTRAINT t10_pkey PRIMARY KEY (rowid ASC), INDEX t9_a_idx (a ASC) WHERE b > 1:::INT8 ) diff --git a/pkg/sql/logictest/testdata/logic_test/virtual_columns b/pkg/sql/logictest/testdata/logic_test/virtual_columns index 10a3cfb53718..2436100567f2 100644 --- a/pkg/sql/logictest/testdata/logic_test/virtual_columns +++ b/pkg/sql/logictest/testdata/logic_test/virtual_columns @@ -1176,7 +1176,7 @@ CREATE TABLE public.t63167_b ( a INT8 NULL, v INT8 NULL AS (a + 1:::INT8) VIRTUAL, rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), - CONSTRAINT t63167_a_pkey PRIMARY KEY (rowid ASC) + CONSTRAINT t63167_b_pkey PRIMARY KEY (rowid ASC) ) # Test that columns backfills to tables with virtual columns work.