From 828853371c6e3b6ec88510dbb99499202516aeba Mon Sep 17 00:00:00 2001 From: Jaewan Park Date: Mon, 20 Jan 2020 21:54:28 +0900 Subject: [PATCH] sql: print comments in SHOW CREATE TABLE Fixes #42875 Release note (sql change): SHOW CREATE TABLE now prints comments. --- pkg/ccl/backupccl/show.go | 6 +- pkg/ccl/backupccl/show_test.go | 19 +++-- pkg/sql/crdb_internal.go | 4 +- .../testdata/logic_test/create_statements | 37 +++++++++ .../logictest/testdata/logic_test/show_create | 33 ++++++++ pkg/sql/show_create.go | 8 +- pkg/sql/show_create_clauses.go | 83 +++++++++++++++++++ pkg/sql/show_create_test.go | 57 ------------- 8 files changed, 178 insertions(+), 69 deletions(-) create mode 100644 pkg/sql/logictest/testdata/logic_test/show_create delete mode 100644 pkg/sql/show_create_test.go diff --git a/pkg/ccl/backupccl/show.go b/pkg/ccl/backupccl/show.go index 90993a5afdf7..02884c2e3e2b 100644 --- a/pkg/ccl/backupccl/show.go +++ b/pkg/ccl/backupccl/show.go @@ -55,7 +55,7 @@ func showBackupPlanHook( case tree.BackupFileDetails: shower = backupShowerFiles default: - shower = backupShowerDefault(ctx, backup.ShouldIncludeSchemas) + shower = backupShowerDefault(ctx, p, backup.ShouldIncludeSchemas) } fn := func(ctx context.Context, _ []sql.PlanNode, resultsCh chan<- tree.Datums) error { @@ -112,7 +112,7 @@ func backupShowerHeaders(showSchemas bool) sqlbase.ResultColumns { return baseHeaders } -func backupShowerDefault(ctx context.Context, showSchemas bool) backupShower { +func backupShowerDefault(ctx context.Context, p sql.PlanHookState, showSchemas bool) backupShower { return backupShower{ header: backupShowerHeaders(showSchemas), fn: func(desc BackupDescriptor) []tree.Datums { @@ -157,7 +157,7 @@ func backupShowerDefault(ctx context.Context, showSchemas bool) backupShower { tree.NewDInt(tree.DInt(descSizes[table.ID].Rows)), } if showSchemas { - schema, err := sql.ShowCreate(ctx, dbName, desc.Descriptors, table, sql.OmitMissingFKClausesFromCreate) + schema, err := sql.ShowCreate(ctx, p, dbName, desc.Descriptors, table, sql.OmitMissingFKClausesFromCreate) if err != nil { continue } diff --git a/pkg/ccl/backupccl/show_test.go b/pkg/ccl/backupccl/show_test.go index 2502c7e5251d..cd7ff5c833f5 100644 --- a/pkg/ccl/backupccl/show_test.go +++ b/pkg/ccl/backupccl/show_test.go @@ -119,17 +119,24 @@ func TestShowBackup(t *testing.T) { // Test that tables, views and sequences are all supported. { viewTableSeq := localFoo + "/tableviewseq" - sqlDB.Exec(t, `CREATE TABLE data.tableA (a int primary key, b int)`) + sqlDB.Exec(t, `CREATE TABLE data.tableA (a int primary key, b int, INDEX tableA_b_idx (b ASC))`) + sqlDB.Exec(t, `COMMENT ON TABLE data.tableA IS 'table'`) + sqlDB.Exec(t, `COMMENT ON COLUMN data.tableA.a IS 'column'`) + sqlDB.Exec(t, `COMMENT ON INDEX data.tableA_b_idx IS 'index'`) sqlDB.Exec(t, `CREATE VIEW data.viewA AS SELECT a from data.tableA`) sqlDB.Exec(t, `CREATE SEQUENCE data.seqA START 1 INCREMENT 2 MAXVALUE 20`) sqlDB.Exec(t, `BACKUP data.tableA, data.viewA, data.seqA TO $1;`, viewTableSeq) expectedCreateTable := `CREATE TABLE tablea ( - a INT8 NOT NULL, - b INT8 NULL, - CONSTRAINT "primary" PRIMARY KEY (a ASC), - FAMILY "primary" (a, b) - )` + a INT8 NOT NULL, + b INT8 NULL, + CONSTRAINT "primary" PRIMARY KEY (a ASC), + INDEX tablea_b_idx (b ASC), + FAMILY "primary" (a, b) +); +COMMENT ON TABLE tablea IS 'table'; +COMMENT ON COLUMN tablea.a IS 'column'; +COMMENT ON INDEX tablea_b_idx IS 'index'` expectedCreateView := `CREATE VIEW viewa (a) AS SELECT a FROM data.public.tablea` expectedCreateSeq := `CREATE SEQUENCE seqa MINVALUE 1 MAXVALUE 20 INCREMENT 2 START 1` diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go index 8524b8a278a1..b39f1c570acf 100644 --- a/pkg/sql/crdb_internal.go +++ b/pkg/sql/crdb_internal.go @@ -1260,7 +1260,7 @@ CREATE TABLE crdb_internal.create_statements ( } else { descType = typeTable tn := (*tree.Name)(&table.Name) - createNofk, err = ShowCreateTable(ctx, tn, contextName, table, lCtx, OmitFKClausesFromCreate) + createNofk, err = ShowCreateTable(ctx, p, tn, contextName, table, lCtx, OmitFKClausesFromCreate) if err != nil { return err } @@ -1268,7 +1268,7 @@ CREATE TABLE crdb_internal.create_statements ( if err := showAlterStatementWithInterleave(ctx, tn, contextName, lCtx, allIdx, table, alterStmts, validateStmts); err != nil { return err } - stmt, err = ShowCreateTable(ctx, tn, contextName, table, lCtx, IncludeFkClausesInCreate) + stmt, err = ShowCreateTable(ctx, p, tn, contextName, table, lCtx, IncludeFkClausesInCreate) } if err != nil { return err diff --git a/pkg/sql/logictest/testdata/logic_test/create_statements b/pkg/sql/logictest/testdata/logic_test/create_statements index 62b596f3ba11..370a3470a3dc 100644 --- a/pkg/sql/logictest/testdata/logic_test/create_statements +++ b/pkg/sql/logictest/testdata/logic_test/create_statements @@ -10,6 +10,24 @@ CREATE TABLE v ( FAMILY "primary" ("'", s, rowid) ) +statement ok +CREATE TABLE c ( + a INT NOT NULL, + b INT NULL, + INDEX c_a_b_idx (a ASC, b ASC), + FAMILY fam_0_a_rowid (a, rowid), + FAMILY fam_1_b (b) +) + +statement ok +COMMENT ON TABLE c IS 'table' + +statement ok +COMMENT ON COLUMN c.a IS 'column' + +statement ok +COMMENT ON INDEX c_a_b_idx IS 'index' + query TTTT colnames SELECT create_statement, create_nofks, alter_statements, validate_statements FROM crdb_internal.create_statements WHERE database_name = 'test' ---- @@ -39,6 +57,25 @@ CREATE TABLE v ( INDEX "v_auto_index_fk_'_ref_t" ("'" ASC), FAMILY "primary" ("'", s, rowid) ) {"ALTER TABLE v ADD CONSTRAINT \"fk_'_ref_t\" FOREIGN KEY (\"'\") REFERENCES t(rowid)","ALTER TABLE v ADD CONSTRAINT fk_s_ref_v FOREIGN KEY (s) REFERENCES v(s)"} {"ALTER TABLE v VALIDATE CONSTRAINT \"fk_'_ref_t\"","ALTER TABLE v VALIDATE CONSTRAINT fk_s_ref_v"} +CREATE TABLE c ( + a INT8 NOT NULL, + b INT8 NULL, + INDEX c_a_b_idx (a ASC, b ASC), + FAMILY fam_0_a_rowid (a, rowid), + FAMILY fam_1_b (b) +); +COMMENT ON TABLE c IS 'table'; +COMMENT ON COLUMN c.a IS 'column'; +COMMENT ON INDEX c_a_b_idx IS 'index' CREATE TABLE c ( + a INT8 NOT NULL, + b INT8 NULL, + INDEX c_a_b_idx (a ASC, b ASC), + FAMILY fam_0_a_rowid (a, rowid), + FAMILY fam_1_b (b) +); +COMMENT ON TABLE c IS 'table'; +COMMENT ON COLUMN c.a IS 'column'; +COMMENT ON INDEX c_a_b_idx IS 'index' {} {} statement error invalid storage parameter "foo" CREATE TABLE a (b INT) WITH (foo=100); diff --git a/pkg/sql/logictest/testdata/logic_test/show_create b/pkg/sql/logictest/testdata/logic_test/show_create new file mode 100644 index 000000000000..ab85daf8bd4d --- /dev/null +++ b/pkg/sql/logictest/testdata/logic_test/show_create @@ -0,0 +1,33 @@ +statement ok +CREATE TABLE c ( + a INT NOT NULL, + b INT NULL, + INDEX c_a_b_idx (a ASC, b ASC), + FAMILY fam_0_a_rowid (a, rowid), + FAMILY fam_1_b (b) +) + +statement ok +COMMENT ON TABLE c IS 'table' + +statement ok +COMMENT ON COLUMN c.a IS 'column' + +statement ok +COMMENT ON INDEX c_a_b_idx IS 'index' + +query TT colnames +SHOW CREATE c +---- +table_name create_statement +c CREATE TABLE c ( + a INT8 NOT NULL, + b INT8 NULL, + INDEX c_a_b_idx (a ASC, b ASC), + FAMILY fam_0_a_rowid (a, rowid), + FAMILY fam_1_b (b) +); +COMMENT ON TABLE c IS 'table'; +COMMENT ON COLUMN c.a IS 'column'; +COMMENT ON INDEX c_a_b_idx IS 'index' + diff --git a/pkg/sql/show_create.go b/pkg/sql/show_create.go index 5cab9d78f1b8..e09da8742298 100644 --- a/pkg/sql/show_create.go +++ b/pkg/sql/show_create.go @@ -47,6 +47,7 @@ const ( // current database. func ShowCreateTable( ctx context.Context, + p PlanHookState, tn *tree.Name, dbPrefix string, desc *sqlbase.TableDescriptor, @@ -150,6 +151,10 @@ func ShowCreateTable( return "", err } + if err := showComment(desc, selectComment(ctx, p, desc.ID), &f.Buffer); err != nil { + return "", err + } + return f.CloseAndGetString(), nil } @@ -175,6 +180,7 @@ func formatQuoteNames(buf *bytes.Buffer, names ...string) { // current database. func ShowCreate( ctx context.Context, + p PlanHookState, dbPrefix string, allDescs []sqlbase.Descriptor, desc *sqlbase.TableDescriptor, @@ -189,7 +195,7 @@ func ShowCreate( stmt, err = ShowCreateSequence(ctx, tn, desc) } else { lCtx := newInternalLookupCtxFromDescriptors(allDescs, nil /* want all tables */) - stmt, err = ShowCreateTable(ctx, tn, dbPrefix, desc, lCtx, ignoreFKs) + stmt, err = ShowCreateTable(ctx, p, tn, dbPrefix, desc, lCtx, ignoreFKs) } return stmt, err diff --git a/pkg/sql/show_create_clauses.go b/pkg/sql/show_create_clauses.go index c7ecdb2bcaca..a367d091a7f7 100644 --- a/pkg/sql/show_create_clauses.go +++ b/pkg/sql/show_create_clauses.go @@ -16,11 +16,60 @@ import ( "fmt" "strings" + "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" + "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/pkg/errors" ) +// tableComment has comment data for table. +type tableComment struct { + comment *string + columns []comment + indexes []comment +} + +type comment struct { + subID int + comment string +} + +// selectComment returns table comment. +func selectComment(ctx context.Context, p PlanHookState, tableID sqlbase.ID) (tc *tableComment) { + query := fmt.Sprintf("SELECT type, object_id, sub_id, comment FROM system.comments WHERE object_id = %d", tableID) + + commentRows, err := p.ExtendedEvalContext().ExecCfg.InternalExecutor.Query( + ctx, "show-tables-with-comment", p.Txn(), query) + if err != nil { + log.VEventf(ctx, 1, "%q", err) + } else { + for _, row := range commentRows { + commentType := int(tree.MustBeDInt(row[0])) + switch commentType { + case keys.TableCommentType, keys.ColumnCommentType, keys.IndexCommentType: + subID := int(tree.MustBeDInt(row[2])) + cmt := string(tree.MustBeDString(row[3])) + + if tc == nil { + tc = &tableComment{} + } + + switch commentType { + case keys.TableCommentType: + tc.comment = &cmt + case keys.ColumnCommentType: + tc.columns = append(tc.columns, comment{subID, cmt}) + case keys.IndexCommentType: + tc.indexes = append(tc.indexes, comment{subID, cmt}) + } + } + } + } + + return tc +} + // ShowCreateView returns a valid SQL representation of the CREATE VIEW // statement used to create the given view. It is used in the implementation of // the crdb_internal.create_statements virtual table. @@ -42,6 +91,40 @@ func ShowCreateView( return f.CloseAndGetString(), nil } +// showComment writes a valid SQL representation of a COMMENT clause. +func showComment(table *sqlbase.TableDescriptor, tc *tableComment, buf *bytes.Buffer) error { + if tc == nil { + return nil + } + + if tc.comment != nil { + buf.WriteString(";\n") + buf.WriteString(fmt.Sprintf("COMMENT ON TABLE %s IS '%s'", table.Name, *tc.comment)) + } + + for _, columnComment := range tc.columns { + col, err := table.FindColumnByID(sqlbase.ColumnID(columnComment.subID)) + if err != nil { + return err + } + + buf.WriteString(";\n") + buf.WriteString(fmt.Sprintf("COMMENT ON COLUMN %s.%s IS '%s'", table.Name, col.Name, columnComment.comment)) + } + + for _, indexComment := range tc.indexes { + idx, err := table.FindIndexByID(sqlbase.IndexID(indexComment.subID)) + if err != nil { + return err + } + + buf.WriteString(";\n") + buf.WriteString(fmt.Sprintf("COMMENT ON INDEX %s IS '%s'", idx.Name, indexComment.comment)) + } + + return nil +} + // showForeignKeyConstraint returns a valid SQL representation of a FOREIGN KEY // clause for a given index. func showForeignKeyConstraint( diff --git a/pkg/sql/show_create_test.go b/pkg/sql/show_create_test.go deleted file mode 100644 index e7ca699058a6..000000000000 --- a/pkg/sql/show_create_test.go +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2018 The Cockroach Authors. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -package sql - -import ( - "context" - "strings" - "testing" - - "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" - "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" - "github.com/cockroachdb/cockroach/pkg/util/leaktest" -) - -func eqWhitespace(a, b string) bool { - return strings.Replace(a, "\t", "", -1) == strings.Replace(b, "\t", "", -1) -} - -func TestStandAloneShowCreateTable(t *testing.T) { - defer leaktest.AfterTest(t)() - - want := `CREATE TABLE jobs ( - id INT8 NOT NULL DEFAULT unique_rowid(), - status STRING NOT NULL, - created TIMESTAMP NOT NULL DEFAULT now():::TIMESTAMP, - payload BYTES NOT NULL, - CONSTRAINT "primary" PRIMARY KEY (id ASC), - CONSTRAINT fk FOREIGN KEY ("???") REFERENCES "[52 as ref]"("???"), - INDEX jobs_status_created_idx (status ASC, created ASC) INTERLEAVE IN PARENT "[51 as parent]" (status), - FAMILY fam_0_id_status_created_payload (id, status, created, payload) - )` - - desc := sqlbase.JobsTable - desc.Indexes = []sqlbase.IndexDescriptor{sqlbase.JobsTable.Indexes[0]} - desc.Indexes[0].Interleave.Ancestors = []sqlbase.InterleaveDescriptor_Ancestor{{TableID: 51, IndexID: 10, SharedPrefixLen: 1}} - desc.OutboundFKs = []sqlbase.ForeignKeyConstraint{{ - ReferencedTableID: 52, - Name: "fk", - }} - - name := tree.Name(desc.Name) - got, err := ShowCreateTable(context.TODO(), &name, "", &desc, nil, IncludeFkClausesInCreate) - if err != nil { - t.Fatal(err) - } - if !eqWhitespace(got, want) { - t.Fatalf("%s, want %s", got, want) - } -}