Skip to content

Commit

Permalink
sql: SHOW CREATE did not show constraint comments
Browse files Browse the repository at this point in the history 
Previously, when using the SHOW CREATE command we
did not show comments create on constraints. To address
this, this patch will extend SHOW CREATE to show constraint
comments. Additionally, this patch addresses an issue where
on primary key swaps constraint IDs were unintentionally
reassigned.

Release note: None
  • Loading branch information
@fqazi
fqazi committed Feb 10, 2022
1 parent bda3564 commit 8dcbe0d
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 21 deletions.
1 change: 1 addition & 0 deletions pkg/sql/catalog/tabledesc/table_desc_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,7 @@ func maybeAddConstraintIDs(desc *descpb.TableDescriptor) (hasChanged bool) {
// we may need to maintain the same constraint ID.
for _, mutation := range desc.GetMutations() {
if idx := mutation.GetIndex(); idx != nil &&
idx.ConstraintID == 0 &&
mutation.Direction == descpb.DescriptorMutation_ADD &&
idx.Unique {
idx.ConstraintID = nextConstraintID()
Expand Down
10 changes: 8 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/alter_primary_key
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1401,13 +1401,17 @@ COMMENT ON INDEX pkey IS 'idx';
COMMENT ON CONSTRAINT pkey ON pkey_comment IS 'const';
CREATE UNIQUE INDEX i2 ON pkey_comment(c);
COMMENT ON INDEX i2 IS 'idx2';
COMMENT ON CONSTRAINT i2 ON pkey_comment IS 'idx3';


# Comment should exist inside the create statement, so filter it out
query T
SELECT substring(@2, strpos(@2, 'COMMENT')) FROM [SHOW CREATE pkey_comment];
----
COMMENT ON INDEX public.pkey_comment@pkey IS 'idx';
COMMENT ON INDEX public.pkey_comment@i2 IS 'idx2'
COMMENT ON INDEX public.pkey_comment@i2 IS 'idx2';
COMMENT ON CONSTRAINT pkey ON public.pkey_comment IS 'const';
COMMENT ON CONSTRAINT i2 ON public.pkey_comment IS 'idx3'


# Altering the primary key should clean up the comment.
Expand All @@ -1419,4 +1423,6 @@ ALTER TABLE pkey_comment ALTER PRIMARY KEY USING COLUMNS (b);
query T
SELECT substring(@2, strpos(@2, 'COMMENT')) FROM [SHOW CREATE pkey_comment];
----
COMMENT ON INDEX public.pkey_comment@i2 IS 'idx2'
COMMENT ON INDEX public.pkey_comment@i2 IS 'idx2';
COMMENT ON CONSTRAINT pkey_comment_a_b_key ON public.pkey_comment IS 'const';
COMMENT ON CONSTRAINT i2 ON public.pkey_comment IS 'idx3'
3 changes: 3 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/show_create_all_tables
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,8 @@ CREATE TABLE test_comment."t t" ("x'" INT PRIMARY KEY);
COMMENT ON TABLE test_comment."t t" IS 'has '' quotes';
COMMENT ON INDEX test_comment."t t"@"t t_pkey" IS 'has '' more '' quotes';
COMMENT ON COLUMN test_comment."t t"."x'" IS 'i '' just '' love '' quotes';
COMMENT ON CONSTRAINT "t t_pkey" ON test_comment."t t" IS 'new constraint comment';


query T colnames
SHOW CREATE ALL TABLES
Expand All @@ -344,6 +346,7 @@ CREATE TABLE public."t t" (
COMMENT ON TABLE public."t t" IS e'has \' quotes';
COMMENT ON COLUMN public."t t"."x'" IS e'i \' just \' love \' quotes';
COMMENT ON INDEX public."t t"@"t t_pkey" IS e'has \' more \' quotes';
COMMENT ON CONSTRAINT "t t_pkey" ON public."t t" IS 'new constraint comment';

# Ensure schemas are shown correctly.
statement ok
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/exec/execbuilder/testdata/show_trace_nonmetamorphic
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ SET tracing = on,kv,results; DROP INDEX t.kv@woo CASCADE; SET tracing = off
query TT
$trace_query
----
batch flow coordinator Put /Table/3/1/108/2/1 -> table:<name:"kv" id:108 version:5 modification_time:<> parent_id:106 unexposed_parent_schema_id:107 columns:<name:"k" id:1 type:<family: IntFamily width: 64 precision: 0 locale: "" visible_type: 0 oid: 20 time_precision_is_set: false > nullable:false hidden:false inaccessible:false generated_as_identity_type:NOT_IDENTITY_COLUMN virtual:false pg_attribute_num:0 alter_column_type_in_progress:false system_column_kind:NONE > columns:<name:"v" id:2 type:<family: IntFamily width: 64 precision: 0 locale: "" visible_type: 0 oid: 20 time_precision_is_set: false > nullable:true hidden:false inaccessible:false generated_as_identity_type:NOT_IDENTITY_COLUMN virtual:false pg_attribute_num:0 alter_column_type_in_progress:false system_column_kind:NONE > next_column_id:3 families:<name:"primary" id:0 column_names:"k" column_names:"v" column_ids:1 column_ids:2 default_column_id:2 > next_family_id:1 primary_index:<name:"kv_pkey" id:1 unique:true version:4 key_column_names:"k" key_column_directions:ASC store_column_names:"v" key_column_ids:1 store_column_ids:2 foreign_key:<table:0 index:0 name:"" validity:Validated shared_prefix_len:0 on_delete:NO_ACTION on_update:NO_ACTION match:SIMPLE > interleave:<> partitioning:<num_columns:0 num_implicit_columns:0 > type:FORWARD created_explicitly:false encoding_type:1 sharded:<is_sharded:false name:"" shard_buckets:0 > disabled:false geo_config:<> predicate:"" use_delete_preserving_encoding:false constraint_id:1 created_at_nanos:... > next_index_id:3 privileges:<users:<user_proto:"admin" privileges:2 with_grant_option:2 > users:<user_proto:"public" privileges:0 with_grant_option:0 > users:<user_proto:"root" privileges:2 with_grant_option:2 > owner_proto:"root" version:2 > mutations:<index:<name:"woo" id:2 unique:true version:3 key_column_names:"v" key_column_directions:ASC key_column_ids:2 key_suffix_column_ids:1 foreign_key:<table:0 index:0 name:"" validity:Validated shared_prefix_len:0 on_delete:NO_ACTION on_update:NO_ACTION match:SIMPLE > interleave:<> partitioning:<num_columns:0 num_implicit_columns:0 > type:FORWARD created_explicitly:true encoding_type:0 sharded:<is_sharded:false name:"" shard_buckets:0 > disabled:false geo_config:<> predicate:"" use_delete_preserving_encoding:false constraint_id:4 created_at_nanos:... > state:DELETE_AND_WRITE_ONLY direction:DROP mutation_id:2 rollback:false > next_mutation_id:3 format_version:3 state:PUBLIC offline_reason:"" view_query:"" is_materialized_view:false mutationJobs:<...> new_schema_change_job_id:0 drop_time:0 replacement_of:<id:0 time:<> > audit_mode:DISABLED drop_job_id:0 create_query:"" create_as_of_time:<...> temporary:false partition_all_by:false exclude_data_from_backup:false next_constraint_id:5 >
batch flow coordinator Put /Table/3/1/108/2/1 -> table:<name:"kv" id:108 version:5 modification_time:<> parent_id:106 unexposed_parent_schema_id:107 columns:<name:"k" id:1 type:<family: IntFamily width: 64 precision: 0 locale: "" visible_type: 0 oid: 20 time_precision_is_set: false > nullable:false hidden:false inaccessible:false generated_as_identity_type:NOT_IDENTITY_COLUMN virtual:false pg_attribute_num:0 alter_column_type_in_progress:false system_column_kind:NONE > columns:<name:"v" id:2 type:<family: IntFamily width: 64 precision: 0 locale: "" visible_type: 0 oid: 20 time_precision_is_set: false > nullable:true hidden:false inaccessible:false generated_as_identity_type:NOT_IDENTITY_COLUMN virtual:false pg_attribute_num:0 alter_column_type_in_progress:false system_column_kind:NONE > next_column_id:3 families:<name:"primary" id:0 column_names:"k" column_names:"v" column_ids:1 column_ids:2 default_column_id:2 > next_family_id:1 primary_index:<name:"kv_pkey" id:1 unique:true version:4 key_column_names:"k" key_column_directions:ASC store_column_names:"v" key_column_ids:1 store_column_ids:2 foreign_key:<table:0 index:0 name:"" validity:Validated shared_prefix_len:0 on_delete:NO_ACTION on_update:NO_ACTION match:SIMPLE > interleave:<> partitioning:<num_columns:0 num_implicit_columns:0 > type:FORWARD created_explicitly:false encoding_type:1 sharded:<is_sharded:false name:"" shard_buckets:0 > disabled:false geo_config:<> predicate:"" use_delete_preserving_encoding:false constraint_id:1 created_at_nanos:... > next_index_id:3 privileges:<users:<user_proto:"admin" privileges:2 with_grant_option:2 > users:<user_proto:"public" privileges:0 with_grant_option:0 > users:<user_proto:"root" privileges:2 with_grant_option:2 > owner_proto:"root" version:2 > mutations:<index:<name:"woo" id:2 unique:true version:3 key_column_names:"v" key_column_directions:ASC key_column_ids:2 key_suffix_column_ids:1 foreign_key:<table:0 index:0 name:"" validity:Validated shared_prefix_len:0 on_delete:NO_ACTION on_update:NO_ACTION match:SIMPLE > interleave:<> partitioning:<num_columns:0 num_implicit_columns:0 > type:FORWARD created_explicitly:true encoding_type:0 sharded:<is_sharded:false name:"" shard_buckets:0 > disabled:false geo_config:<> predicate:"" use_delete_preserving_encoding:false constraint_id:2 created_at_nanos:... > state:DELETE_AND_WRITE_ONLY direction:DROP mutation_id:2 rollback:false > next_mutation_id:3 format_version:3 state:PUBLIC offline_reason:"" view_query:"" is_materialized_view:false mutationJobs:<...> new_schema_change_job_id:0 drop_time:0 replacement_of:<id:0 time:<> > audit_mode:DISABLED drop_job_id:0 create_query:"" create_as_of_time:<...> temporary:false partition_all_by:false exclude_data_from_backup:false next_constraint_id:3 >
sql query rows affected: 0

statement ok
Expand All @@ -183,7 +183,7 @@ query TT
$trace_query
----
batch flow coordinator Del /NamespaceTable/30/1/106/107/"kv"/4/1
batch flow coordinator Put /Table/3/1/108/2/1 -> table:<name:"kv" id:108 version:8 modification_time:<> parent_id:106 unexposed_parent_schema_id:107 columns:<name:"k" id:1 type:<family: IntFamily width: 64 precision: 0 locale: "" visible_type: 0 oid: 20 time_precision_is_set: false > nullable:false hidden:false inaccessible:false generated_as_identity_type:NOT_IDENTITY_COLUMN virtual:false pg_attribute_num:0 alter_column_type_in_progress:false system_column_kind:NONE > columns:<name:"v" id:2 type:<family: IntFamily width: 64 precision: 0 locale: "" visible_type: 0 oid: 20 time_precision_is_set: false > nullable:true hidden:false inaccessible:false generated_as_identity_type:NOT_IDENTITY_COLUMN virtual:false pg_attribute_num:0 alter_column_type_in_progress:false system_column_kind:NONE > next_column_id:3 families:<name:"primary" id:0 column_names:"k" column_names:"v" column_ids:1 column_ids:2 default_column_id:2 > next_family_id:1 primary_index:<name:"kv_pkey" id:1 unique:true version:4 key_column_names:"k" key_column_directions:ASC store_column_names:"v" key_column_ids:1 store_column_ids:2 foreign_key:<table:0 index:0 name:"" validity:Validated shared_prefix_len:0 on_delete:NO_ACTION on_update:NO_ACTION match:SIMPLE > interleave:<> partitioning:<num_columns:0 num_implicit_columns:0 > type:FORWARD created_explicitly:false encoding_type:1 sharded:<is_sharded:false name:"" shard_buckets:0 > disabled:false geo_config:<> predicate:"" use_delete_preserving_encoding:false constraint_id:1 created_at_nanos:... > next_index_id:3 privileges:<users:<user_proto:"admin" privileges:2 with_grant_option:2 > users:<user_proto:"public" privileges:0 with_grant_option:0 > users:<user_proto:"root" privileges:2 with_grant_option:2 > owner_proto:"root" version:2 > next_mutation_id:3 format_version:3 state:DROP offline_reason:"" view_query:"" is_materialized_view:false new_schema_change_job_id:0 drop_time:... replacement_of:<id:0 time:<> > audit_mode:DISABLED drop_job_id:0 create_query:"" create_as_of_time:<...> temporary:false partition_all_by:false exclude_data_from_backup:false next_constraint_id:5 >
batch flow coordinator Put /Table/3/1/108/2/1 -> table:<name:"kv" id:108 version:8 modification_time:<> parent_id:106 unexposed_parent_schema_id:107 columns:<name:"k" id:1 type:<family: IntFamily width: 64 precision: 0 locale: "" visible_type: 0 oid: 20 time_precision_is_set: false > nullable:false hidden:false inaccessible:false generated_as_identity_type:NOT_IDENTITY_COLUMN virtual:false pg_attribute_num:0 alter_column_type_in_progress:false system_column_kind:NONE > columns:<name:"v" id:2 type:<family: IntFamily width: 64 precision: 0 locale: "" visible_type: 0 oid: 20 time_precision_is_set: false > nullable:true hidden:false inaccessible:false generated_as_identity_type:NOT_IDENTITY_COLUMN virtual:false pg_attribute_num:0 alter_column_type_in_progress:false system_column_kind:NONE > next_column_id:3 families:<name:"primary" id:0 column_names:"k" column_names:"v" column_ids:1 column_ids:2 default_column_id:2 > next_family_id:1 primary_index:<name:"kv_pkey" id:1 unique:true version:4 key_column_names:"k" key_column_directions:ASC store_column_names:"v" key_column_ids:1 store_column_ids:2 foreign_key:<table:0 index:0 name:"" validity:Validated shared_prefix_len:0 on_delete:NO_ACTION on_update:NO_ACTION match:SIMPLE > interleave:<> partitioning:<num_columns:0 num_implicit_columns:0 > type:FORWARD created_explicitly:false encoding_type:1 sharded:<is_sharded:false name:"" shard_buckets:0 > disabled:false geo_config:<> predicate:"" use_delete_preserving_encoding:false constraint_id:1 created_at_nanos:... > next_index_id:3 privileges:<users:<user_proto:"admin" privileges:2 with_grant_option:2 > users:<user_proto:"public" privileges:0 with_grant_option:0 > users:<user_proto:"root" privileges:2 with_grant_option:2 > owner_proto:"root" version:2 > next_mutation_id:3 format_version:3 state:DROP offline_reason:"" view_query:"" is_materialized_view:false new_schema_change_job_id:0 drop_time:... replacement_of:<id:0 time:<> > audit_mode:DISABLED drop_job_id:0 create_query:"" create_as_of_time:<...> temporary:false partition_all_by:false exclude_data_from_backup:false next_constraint_id:3 >
sql query rows affected: 0

# Check that session tracing does not inhibit the fast path for inserts &
Expand Down
15 changes: 2 additions & 13 deletions pkg/sql/schema_changer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,7 @@ func WaitToUpdateLeases(
//
// It also kicks off GC jobs as needed.
func (sc *SchemaChanger) done(ctx context.Context) error {
// Gathers ant comments that need to be cleaned up.
// Gathers ant comments that need to be swapped/cleaned.
type commentToDelete struct {
id int64
subID int64
Expand Down Expand Up @@ -1205,23 +1205,12 @@ func (sc *SchemaChanger) done(ctx context.Context) error {
subID: int64(id),
commentType: keys.IndexCommentType,
})
for _, idx := range scTable.AllIndexes() {
if idx.GetID() == id {
commentsToDelete = append(commentsToDelete,
commentToDelete{
id: int64(scTable.GetID()),
subID: int64(idx.GetConstraintID()),
commentType: keys.ConstraintCommentType,
})
break
}
}
for i := range pkSwap.PrimaryKeySwapDesc().OldIndexes {
// Skip the primary index.
if pkSwap.PrimaryKeySwapDesc().OldIndexes[i] == id {
continue
}
// Setup a swap operation for each new index
// Set up a swap operation for any re-created indexes.
commentsToSwap = append(commentsToSwap,
commentToSwap{
id: int64(scTable.GetID()),
Expand Down
31 changes: 27 additions & 4 deletions pkg/sql/show_create_clauses.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ import (

// tableComments stores the comment data for a table.
type tableComments struct {
comment *string
columns []comment
indexes []comment
comment *string
columns []comment
indexes []comment
constraints []comment
}

type comment struct {
Expand All @@ -60,7 +61,8 @@ func selectComment(ctx context.Context, p PlanHookState, tableID descpb.ID) (tc
row := it.Cur()
commentType := keys.CommentType(tree.MustBeDInt(row[0]))
switch commentType {
case keys.TableCommentType, keys.ColumnCommentType, keys.IndexCommentType:
case keys.TableCommentType, keys.ColumnCommentType,
keys.IndexCommentType, keys.ConstraintCommentType:
subID := int(tree.MustBeDInt(row[2]))
cmt := string(tree.MustBeDString(row[3]))

Expand All @@ -75,6 +77,8 @@ func selectComment(ctx context.Context, p PlanHookState, tableID descpb.ID) (tc
tc.columns = append(tc.columns, comment{subID, cmt})
case keys.IndexCommentType:
tc.indexes = append(tc.indexes, comment{subID, cmt})
case keys.ConstraintCommentType:
tc.constraints = append(tc.constraints, comment{subID, cmt})
}
}
}
Expand Down Expand Up @@ -295,6 +299,25 @@ func showComments(
})
}

// Get all the constraints for the table and create a map by ID.
constraints, err := table.GetConstraintInfo()
if err != nil {
return err
}
constraintIDToConstraint := make(map[descpb.ConstraintID]string)
for constraintName, constraint := range constraints {
constraintIDToConstraint[constraint.ConstraintID] = constraintName
}
for _, constraintComment := range tc.constraints {
f.WriteString(";\n")
constraintName := constraintIDToConstraint[descpb.ConstraintID(constraintComment.subID)]
f.FormatNode(&tree.CommentOnConstraint{
Constraint: tree.Name(constraintName),
Table: tn.ToUnresolvedObjectName(),
Comment: &constraintComment.comment,
})
}

buf.WriteString(f.CloseAndGetString())
return nil
}
Expand Down

0 comments on commit 8dcbe0d

Please sign in to comment.