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

sql: fix CREATE TABLE LIKE with implicit pk #82555

Merged
merged 1 commit into from
Jun 9, 2022
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
47 changes: 34 additions & 13 deletions pkg/sql/create_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}{}
Expand All @@ -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{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
13 changes: 6 additions & 7 deletions pkg/sql/logictest/testdata/logic_test/create_table
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
)

Expand All @@ -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)
)

Expand All @@ -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)
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/partial_index
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/virtual_columns
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down