From 672b4e8e06ccbfd94b7741babde5fb3204bb6dd9 Mon Sep 17 00:00:00 2001 From: Jaewan Park Date: Sat, 24 Nov 2018 09:13:44 +0900 Subject: [PATCH] sql: implement `COMMENT ON TABLE` See #19472 Release note (sql change): support COMMENT ON TABLE syntax --- docs/generated/sql/bnf/stmt_block.bnf | 8 ++ pkg/cli/cli_test.go | 2 + pkg/keys/constants.go | 4 + pkg/sql/comment_on_table.go | 110 ++++++++++++++++++ pkg/sql/event_log.go | 2 + pkg/sql/expand_plan.go | 2 + .../logictest/testdata/logic_test/grant_table | 15 +++ .../testdata/logic_test/information_schema | 29 +++++ .../logictest/testdata/logic_test/pg_catalog | 4 + .../logictest/testdata/logic_test/show_source | 1 + pkg/sql/logictest/testdata/logic_test/system | 13 +++ .../logictest/testdata/planner_test/explain | 2 +- pkg/sql/opt/exec/execbuilder/testdata/explain | 2 +- pkg/sql/opt_filters.go | 1 + pkg/sql/opt_limits.go | 1 + pkg/sql/opt_needed.go | 1 + pkg/sql/parser/parse_test.go | 4 +- pkg/sql/parser/sql.y | 20 +++- pkg/sql/pg_catalog.go | 27 ++++- pkg/sql/pgwire/pgwire_test.go | 2 +- pkg/sql/plan.go | 2 + pkg/sql/sem/builtins/pg_builtins.go | 19 ++- pkg/sql/sem/tree/comment_on_table.go | 35 ++++++ pkg/sql/sem/tree/stmt.go | 7 ++ pkg/sql/sqlbase/system.go | 43 +++++++ pkg/sql/tests/system_table_test.go | 1 + pkg/sql/walk.go | 1 + pkg/sqlmigrations/migrations.go | 11 ++ 28 files changed, 357 insertions(+), 12 deletions(-) create mode 100644 pkg/sql/comment_on_table.go create mode 100644 pkg/sql/sem/tree/comment_on_table.go diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf index c1534c70a4e6..ec528a926e51 100644 --- a/docs/generated/sql/bnf/stmt_block.bnf +++ b/docs/generated/sql/bnf/stmt_block.bnf @@ -8,6 +8,7 @@ stmt ::= 'HELPTOKEN' | preparable_stmt | copy_from_stmt + | comment_stmt | execute_stmt | deallocate_stmt | discard_stmt @@ -46,6 +47,9 @@ preparable_stmt ::= copy_from_stmt ::= 'COPY' table_name opt_column_list 'FROM' 'STDIN' +comment_stmt ::= + 'COMMENT' 'ON' 'TABLE' table_name 'IS' comment_text + execute_stmt ::= 'EXECUTE' table_alias_name execute_param_clause @@ -196,6 +200,10 @@ opt_column_list ::= '(' name_list ')' | +comment_text ::= + 'SCONST' + | 'NULL' + table_alias_name ::= name diff --git a/pkg/cli/cli_test.go b/pkg/cli/cli_test.go index 1cac8ab48c2f..68db285443ff 100644 --- a/pkg/cli/cli_test.go +++ b/pkg/cli/cli_test.go @@ -2334,10 +2334,12 @@ writing ` + os.DevNull + ` debug/nodes/1/ranges/18 debug/nodes/1/ranges/19 debug/nodes/1/ranges/20 + debug/nodes/1/ranges/21 debug/reports/problemranges debug/schema/defaultdb@details debug/schema/postgres@details debug/schema/system@details + debug/schema/system/comments debug/schema/system/descriptor debug/schema/system/eventlog debug/schema/system/jobs diff --git a/pkg/keys/constants.go b/pkg/keys/constants.go index 2d29ed116728..f5ba7d8ac49a 100644 --- a/pkg/keys/constants.go +++ b/pkg/keys/constants.go @@ -328,4 +328,8 @@ const ( LocationsTableID = 21 LivenessRangesID = 22 RoleMembersTableID = 23 + CommentsTableID = 24 + + // CommentType is type for system.comments + TableCommentType = 1 ) diff --git a/pkg/sql/comment_on_table.go b/pkg/sql/comment_on_table.go new file mode 100644 index 000000000000..e4a69cc59535 --- /dev/null +++ b/pkg/sql/comment_on_table.go @@ -0,0 +1,110 @@ +// Copyright 2018 The Cockroach Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +// implied. See the License for the specific language governing +// permissions and limitations under the License. + +package sql + +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/sql/privilege" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" +) + +type commentOnTableNode struct { + n *tree.CommentOnTable + tableDesc *MutableTableDescriptor + dbDesc *sqlbase.DatabaseDescriptor +} + +// CommentOnTable add comment on a table. +// Privileges: CREATE on table. +// notes: postgres requires CREATE on the table. +// mysql requires ALTER, CREATE, INSERT on the table. +func (p *planner) CommentOnTable(ctx context.Context, n *tree.CommentOnTable) (planNode, error) { + tableDesc, err := p.ResolveMutableTableDescriptor(ctx, &n.Table, true, requireTableDesc) + if err != nil { + return nil, err + } + if tableDesc == nil { + return newZeroNode(nil /* columns */), nil + } + + if err := p.CheckPrivilege(ctx, tableDesc, privilege.CREATE); err != nil { + return nil, err + } + + dbDesc, err := getDatabaseDescByID( + ctx, + p.Txn(), + tableDesc.ParentID) + if err != nil { + return nil, err + } + + return &commentOnTableNode{n: n, tableDesc: tableDesc, dbDesc: dbDesc}, nil +} + +func (n *commentOnTableNode) startExec(params runParams) error { + h := makeOidHasher() + oid := h.TableOid(n.dbDesc, tree.PublicSchema, n.tableDesc.TableDesc()) + + if n.n.Comment != nil { + _, err := params.p.extendedEvalCtx.ExecCfg.InternalExecutor.Exec( + params.ctx, + "upsert-comment", + params.p.Txn(), + "UPSERT INTO system.comments VALUES ($1, $2, 0, $3)", + keys.TableCommentType, + oid.DInt, + *n.n.Comment) + if err != nil { + return err + } + } else { + _, err := params.p.extendedEvalCtx.ExecCfg.InternalExecutor.Exec( + params.ctx, + "delete-comment", + params.p.Txn(), + "DELETE FROM system.comments WHERE type=$1 AND object_id=$2 AND sub_id=0", + keys.TableCommentType, + oid.DInt) + if err != nil { + return err + } + } + + return MakeEventLogger(params.extendedEvalCtx.ExecCfg).InsertEventRecord( + params.ctx, + params.p.txn, + EventLogCommentOnTable, + int32(n.tableDesc.ID), + int32(params.extendedEvalCtx.NodeID), + struct { + TableName string + Statement string + User string + Comment *string + }{ + n.n.Table.FQString(), + n.n.String(), + params.SessionData().User, + n.n.Comment}, + ) +} + +func (n *commentOnTableNode) Next(runParams) (bool, error) { return false, nil } +func (n *commentOnTableNode) Values() tree.Datums { return tree.Datums{} } +func (n *commentOnTableNode) Close(context.Context) {} diff --git a/pkg/sql/event_log.go b/pkg/sql/event_log.go index 95a973e70e6a..03a7bdfb76fe 100644 --- a/pkg/sql/event_log.go +++ b/pkg/sql/event_log.go @@ -44,6 +44,8 @@ const ( EventLogTruncateTable EventLogType = "truncate_table" // EventLogAlterTable is recorded when a table is altered. EventLogAlterTable EventLogType = "alter_table" + // EventLogCommentOnTable is recorded when a table is commented. + EventLogCommentOnTable EventLogType = "comment_on_table" // EventLogCreateIndex is recorded when an index is created. EventLogCreateIndex EventLogType = "create_index" diff --git a/pkg/sql/expand_plan.go b/pkg/sql/expand_plan.go index fbb38522e1ec..974587d3b137 100644 --- a/pkg/sql/expand_plan.go +++ b/pkg/sql/expand_plan.go @@ -347,6 +347,7 @@ func doExpandPlan( case *alterTableNode: case *alterSequenceNode: case *alterUserSetPasswordNode: + case *commentOnTableNode: case *renameColumnNode: case *renameDatabaseNode: case *renameIndexNode: @@ -854,6 +855,7 @@ func (p *planner) simplifyOrderings(plan planNode, usefulOrdering sqlbase.Column case *alterTableNode: case *alterSequenceNode: case *alterUserSetPasswordNode: + case *commentOnTableNode: case *renameColumnNode: case *renameDatabaseNode: case *renameIndexNode: diff --git a/pkg/sql/logictest/testdata/logic_test/grant_table b/pkg/sql/logictest/testdata/logic_test/grant_table index 95f80f2ff42b..77c1274ff4e8 100644 --- a/pkg/sql/logictest/testdata/logic_test/grant_table +++ b/pkg/sql/logictest/testdata/logic_test/grant_table @@ -160,6 +160,16 @@ system public NULL admin GRANT system public NULL admin SELECT system public NULL root GRANT system public NULL root SELECT +system public comments admin DELETE +system public comments admin GRANT +system public comments admin INSERT +system public comments admin SELECT +system public comments admin UPDATE +system public comments root DELETE +system public comments root GRANT +system public comments root INSERT +system public comments root SELECT +system public comments root UPDATE system public descriptor admin GRANT system public descriptor admin SELECT system public descriptor root GRANT @@ -315,6 +325,11 @@ system pg_catalog NULL root GRANT system pg_catalog NULL root SELECT system public NULL root GRANT system public NULL root SELECT +system public comments root DELETE +system public comments root GRANT +system public comments root INSERT +system public comments root SELECT +system public comments root UPDATE system public descriptor root GRANT system public descriptor root SELECT system public eventlog root DELETE diff --git a/pkg/sql/logictest/testdata/logic_test/information_schema b/pkg/sql/logictest/testdata/logic_test/information_schema index 5509a909d0a0..92498a5dc6df 100644 --- a/pkg/sql/logictest/testdata/logic_test/information_schema +++ b/pkg/sql/logictest/testdata/logic_test/information_schema @@ -575,6 +575,7 @@ system public web_sessions BASE TABLE system public table_statistics BASE TABLE YES 1 system public locations BASE TABLE YES 1 system public role_members BASE TABLE YES 1 +system public comments BASE TABLE YES 1 statement ok ALTER TABLE other_db.xyz ADD COLUMN j INT @@ -627,6 +628,7 @@ FROM system.information_schema.table_constraints ORDER BY TABLE_NAME, CONSTRAINT_TYPE, CONSTRAINT_NAME ---- constraint_catalog constraint_schema constraint_name table_catalog table_schema table_name constraint_type is_deferrable initially_deferred +system public primary system public comments PRIMARY KEY NO NO system public primary system public descriptor PRIMARY KEY NO NO system public primary system public eventlog PRIMARY KEY NO NO system public primary system public jobs PRIMARY KEY NO NO @@ -648,6 +650,9 @@ FROM system.information_schema.constraint_column_usage ORDER BY TABLE_NAME, COLUMN_NAME, CONSTRAINT_NAME ---- table_catalog table_schema table_name column_name constraint_catalog constraint_schema constraint_name +system public comments object_id system public primary +system public comments sub_id system public primary +system public comments type system public primary system public descriptor id system public primary system public eventlog timestamp system public primary system public eventlog uniqueID system public primary @@ -729,6 +734,10 @@ WHERE table_schema != 'information_schema' AND table_schema != 'pg_catalog' AND ORDER BY 3,4 ---- table_catalog table_schema table_name column_name ordinal_position +system public comments comment 4 +system public comments object_id 2 +system public comments sub_id 3 +system public comments type 1 system public descriptor descriptor 2 system public descriptor id 1 system public eventlog eventType 2 @@ -1130,6 +1139,16 @@ NULL public system pg_catalog pg_type NULL public system pg_catalog pg_user SELECT NULL NULL NULL public system pg_catalog pg_user_mapping SELECT NULL NULL NULL public system pg_catalog pg_views SELECT NULL NULL +NULL admin system public comments DELETE NULL NULL +NULL admin system public comments GRANT NULL NULL +NULL admin system public comments INSERT NULL NULL +NULL admin system public comments SELECT NULL NULL +NULL admin system public comments UPDATE NULL NULL +NULL root system public comments DELETE NULL NULL +NULL root system public comments GRANT NULL NULL +NULL root system public comments INSERT NULL NULL +NULL root system public comments SELECT NULL NULL +NULL root system public comments UPDATE NULL NULL NULL admin system public descriptor GRANT NULL NULL NULL admin system public descriptor SELECT NULL NULL NULL root system public descriptor GRANT NULL NULL @@ -1482,6 +1501,16 @@ NULL root system public role_members NULL root system public role_members INSERT NULL NULL NULL root system public role_members SELECT NULL NULL NULL root system public role_members UPDATE NULL NULL +NULL admin system public comments DELETE NULL NULL +NULL admin system public comments GRANT NULL NULL +NULL admin system public comments INSERT NULL NULL +NULL admin system public comments SELECT NULL NULL +NULL admin system public comments UPDATE NULL NULL +NULL root system public comments DELETE NULL NULL +NULL root system public comments GRANT NULL NULL +NULL root system public comments INSERT NULL NULL +NULL root system public comments SELECT NULL NULL +NULL root system public comments UPDATE NULL NULL statement ok CREATE TABLE other_db.xyz (i INT) diff --git a/pkg/sql/logictest/testdata/logic_test/pg_catalog b/pkg/sql/logictest/testdata/logic_test/pg_catalog index b86d65f64a5d..644b58c796c7 100644 --- a/pkg/sql/logictest/testdata/logic_test/pg_catalog +++ b/pkg/sql/logictest/testdata/logic_test/pg_catalog @@ -591,6 +591,7 @@ indexrelid indrelid indnatts indisunique indisprimary indisexclusion ind 633004885 1455563232 1 false false false false false true false false true false 5 0 0 0 NULL NULL 633004886 1455563232 1 true true false true false true false false true false 1 0 0 0 NULL NULL 961325075 2492394317 1 true true false true false true false false true false 1 0 0 0 NULL NULL +1112802205 3710069875 3 true true false true false true false false true false 1 2 3 0 0 0 0 0 0 0 0 0 NULL NULL 1168848597 1038690067 2 true true false true false true false false true false 1 7 0 0 0 0 0 0 NULL NULL 1849259112 3649853378 1 true true false true false true false false true false 1 0 0 0 NULL NULL 1849259115 3649853378 2 false false false false false true false false true false 2 3 1661428263 0 0 0 0 0 NULL NULL @@ -623,6 +624,9 @@ indexrelid operator_argument_type_oid operator_argument_position 633004885 0 1 633004886 0 1 961325075 0 1 +1112802205 0 1 +1112802205 0 2 +1112802205 0 3 1168848597 0 1 1168848597 0 2 1849259112 0 1 diff --git a/pkg/sql/logictest/testdata/logic_test/show_source b/pkg/sql/logictest/testdata/logic_test/show_source index 36147944aa88..883b327f9cd6 100644 --- a/pkg/sql/logictest/testdata/logic_test/show_source +++ b/pkg/sql/logictest/testdata/logic_test/show_source @@ -142,6 +142,7 @@ query T colnames SELECT * FROM [SHOW TABLES FROM system] ---- table_name +comments descriptor eventlog jobs diff --git a/pkg/sql/logictest/testdata/logic_test/system b/pkg/sql/logictest/testdata/logic_test/system index fc8c62426162..06f641fb2e50 100644 --- a/pkg/sql/logictest/testdata/logic_test/system +++ b/pkg/sql/logictest/testdata/logic_test/system @@ -11,6 +11,7 @@ test query T SHOW TABLES FROM system ---- +comments descriptor eventlog jobs @@ -33,6 +34,7 @@ SELECT * FROM system.namespace 0 postgres 51 0 system 1 0 test 52 +1 comments 24 1 descriptor 3 1 eventlog 12 1 jobs 15 @@ -66,6 +68,7 @@ SELECT id FROM system.descriptor 20 21 23 +24 50 51 52 @@ -188,6 +191,16 @@ system public root SELECT query TTTTT SHOW GRANTS ON system.* ---- +system public comments admin DELETE +system public comments admin GRANT +system public comments admin INSERT +system public comments admin SELECT +system public comments admin UPDATE +system public comments root DELETE +system public comments root GRANT +system public comments root INSERT +system public comments root SELECT +system public comments root UPDATE system public descriptor admin GRANT system public descriptor admin SELECT system public descriptor root GRANT diff --git a/pkg/sql/logictest/testdata/planner_test/explain b/pkg/sql/logictest/testdata/planner_test/explain index 2ac8e482b730..0251a33cdc54 100644 --- a/pkg/sql/logictest/testdata/planner_test/explain +++ b/pkg/sql/logictest/testdata/planner_test/explain @@ -249,7 +249,7 @@ sort · · └── filter · · │ filter (table_catalog, table_schema, table_name) IN (('test', 'public', 'foo'),) └── values · · -· size 8 columns, 585 rows +· size 8 columns, 595 rows query TTT diff --git a/pkg/sql/opt/exec/execbuilder/testdata/explain b/pkg/sql/opt/exec/execbuilder/testdata/explain index 4309474052ce..cd758b81f08e 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/explain +++ b/pkg/sql/opt/exec/execbuilder/testdata/explain @@ -240,7 +240,7 @@ sort · · └── filter · · │ filter (table_catalog, table_schema, table_name) IN (('test', 'public', 'foo'),) └── values · · -· size 8 columns, 585 rows +· size 8 columns, 595 rows query TTT diff --git a/pkg/sql/opt_filters.go b/pkg/sql/opt_filters.go index 0243e8180a9e..13916d038949 100644 --- a/pkg/sql/opt_filters.go +++ b/pkg/sql/opt_filters.go @@ -366,6 +366,7 @@ func (p *planner) propagateFilters( case *renameTableNode: case *scrubNode: case *truncateNode: + case *commentOnTableNode: case *createDatabaseNode: case *createIndexNode: case *CreateUserNode: diff --git a/pkg/sql/opt_limits.go b/pkg/sql/opt_limits.go index 3b9515581107..6fc62805b0e1 100644 --- a/pkg/sql/opt_limits.go +++ b/pkg/sql/opt_limits.go @@ -213,6 +213,7 @@ func (p *planner) applyLimit(plan planNode, numRows int64, soft bool) { case *renameTableNode: case *scrubNode: case *truncateNode: + case *commentOnTableNode: case *createDatabaseNode: case *createIndexNode: case *CreateUserNode: diff --git a/pkg/sql/opt_needed.go b/pkg/sql/opt_needed.go index d9a6a1e0c623..b51258af5cd7 100644 --- a/pkg/sql/opt_needed.go +++ b/pkg/sql/opt_needed.go @@ -261,6 +261,7 @@ func setNeededColumns(plan planNode, needed []bool) { case *renameTableNode: case *scrubNode: case *truncateNode: + case *commentOnTableNode: case *createDatabaseNode: case *createIndexNode: case *CreateUserNode: diff --git a/pkg/sql/parser/parse_test.go b/pkg/sql/parser/parse_test.go index 8225eb409efd..44605d338f89 100644 --- a/pkg/sql/parser/parse_test.go +++ b/pkg/sql/parser/parse_test.go @@ -1127,6 +1127,9 @@ func TestParse(t *testing.T) { {`EXPLAIN ALTER TABLE t EXPERIMENTAL_AUDIT SET READ WRITE`}, {`ALTER TABLE t EXPERIMENTAL_AUDIT SET OFF`}, + {`COMMENT ON TABLE foo IS 'a'`}, + {`COMMENT ON TABLE foo IS NULL`}, + {`ALTER SEQUENCE a RENAME TO b`}, {`EXPLAIN ALTER SEQUENCE a RENAME TO b`}, {`ALTER SEQUENCE IF EXISTS a RENAME TO b`}, @@ -2575,7 +2578,6 @@ func TestUnimplementedSyntax(t *testing.T) { {`COMMENT ON COLUMN a.b IS 'a'`, 19472, `column`}, {`COMMENT ON DATABASE a IS 'b'`, 19472, ``}, - {`COMMENT ON TABLE foo IS 'a'`, 19472, `table`}, {`CREATE AGGREGATE a`, 0, `create aggregate`}, {`CREATE CAST a`, 0, `create cast`}, diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index bb037bd25f60..e52a1e8f23eb 100644 --- a/pkg/sql/parser/sql.y +++ b/pkg/sql/parser/sql.y @@ -722,7 +722,7 @@ func newNameFromStr(s string) *tree.Name { %type show_zone_stmt %type session_var -%type comment_text +%type <*string> comment_text %type transaction_stmt %type truncate_stmt @@ -2007,7 +2007,12 @@ cancel_sessions_stmt: comment_stmt: COMMENT ON TABLE table_name IS comment_text { - return unimplementedWithIssueDetail(sqllex, 19472, "table") + name, err := tree.NormalizeTableName($4.unresolvedName()) + if err != nil { + sqllex.Error(err.Error()) + return 1 + } + $$.val = &tree.CommentOnTable{Table: name, Comment: $6.strPtr()} } | COMMENT ON COLUMN column_path IS comment_text { @@ -2019,8 +2024,15 @@ comment_stmt: } comment_text: - SCONST { $$ = $1 } - | NULL { $$ = "" } + SCONST + { + $$.val = &$1 + } +| NULL + { + var str *string + $$.val = str + } // %Help: CREATE // %Category: Group diff --git a/pkg/sql/pg_catalog.go b/pkg/sql/pg_catalog.go index 141ebe351cbc..91487d016d9e 100644 --- a/pkg/sql/pg_catalog.go +++ b/pkg/sql/pg_catalog.go @@ -941,8 +941,31 @@ CREATE TABLE pg_catalog.pg_description ( description STRING ); `, - populate: func(_ context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error { - // Comments on database objects are not currently supported. + populate: func( + ctx context.Context, + p *planner, + dbContext *DatabaseDescriptor, + addRow func(...tree.Datum) error) error { + + comments, _, err := p.extendedEvalCtx.ExecCfg.InternalExecutor.Query( + ctx, + "select-comments", + p.EvalContext().Txn, + "SELECT object_id, sub_id, comment FROM system.comments") + if err != nil { + return err + } + + for _, comment := range comments { + if err := addRow( + tree.NewDOid(*comment[0].(*tree.DInt)), + oidZero, + comment[1], + comment[2]); err != nil { + return err + } + } + return nil }, } diff --git a/pkg/sql/pgwire/pgwire_test.go b/pkg/sql/pgwire/pgwire_test.go index e3903cf521a1..333b5f51bdb6 100644 --- a/pkg/sql/pgwire/pgwire_test.go +++ b/pkg/sql/pgwire/pgwire_test.go @@ -734,7 +734,7 @@ func TestPGPreparedQuery(t *testing.T) { baseTest.Results("users", "primary", false, 1, "username", "ASC", false, false), }}, {"SHOW TABLES FROM system", []preparedQueryTest{ - baseTest.Results("descriptor").Others(13), + baseTest.Results("comments").Others(14), }}, {"SHOW SCHEMAS FROM system", []preparedQueryTest{ baseTest.Results("crdb_internal").Others(3), diff --git a/pkg/sql/plan.go b/pkg/sql/plan.go index 79a753f79fb1..ae10fca24d04 100644 --- a/pkg/sql/plan.go +++ b/pkg/sql/plan.go @@ -773,6 +773,8 @@ func (p *planner) newPlan( return p.CancelQueries(ctx, n) case *tree.CancelSessions: return p.CancelSessions(ctx, n) + case *tree.CommentOnTable: + return p.CommentOnTable(ctx, n) case *tree.ControlJobs: return p.ControlJobs(ctx, n) case *tree.Scrub: diff --git a/pkg/sql/sem/builtins/pg_builtins.go b/pkg/sql/sem/builtins/pg_builtins.go index c8dd2832a542..cdc8dff9fc21 100644 --- a/pkg/sql/sem/builtins/pg_builtins.go +++ b/pkg/sql/sem/builtins/pg_builtins.go @@ -724,8 +724,23 @@ var pgBuiltins = map[string]builtinDefinition{ tree.Overload{ Types: tree.ArgTypes{{"object_oid", types.Oid}}, ReturnType: tree.FixedReturnType(types.String), - Fn: func(_ *tree.EvalContext, _ tree.Datums) (tree.Datum, error) { - return tree.DNull, nil + Fn: func(ctx *tree.EvalContext, args tree.Datums) (tree.Datum, error) { + oid, ok := args[0].(*tree.DOid) + if !ok { + return tree.DNull, nil + } + + r, err := ctx.InternalExecutor.QueryRow( + ctx.Ctx(), "pg_get_objdesc", + ctx.Txn, + "SELECT comment FROM system.comments WHERE object_id=$1 LIMIT 1", oid.DInt) + if err != nil { + return nil, err + } + if len(r) == 0 { + return tree.DNull, nil + } + return r[0], nil }, Info: notUsableInfo, }, diff --git a/pkg/sql/sem/tree/comment_on_table.go b/pkg/sql/sem/tree/comment_on_table.go new file mode 100644 index 000000000000..1d38c56e9ac6 --- /dev/null +++ b/pkg/sql/sem/tree/comment_on_table.go @@ -0,0 +1,35 @@ +// Copyright 2018 The Cockroach Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +// implied. See the License for the specific language governing +// permissions and limitations under the License. + +package tree + +import "github.com/cockroachdb/cockroach/pkg/sql/lex" + +// CommentOnTable represents an COMMENT ON TABLE statement. +type CommentOnTable struct { + Table TableName + Comment *string +} + +// Format implements the NodeFormatter interface. +func (n *CommentOnTable) Format(ctx *FmtCtx) { + ctx.WriteString("COMMENT ON TABLE ") + ctx.FormatNode(&n.Table) + ctx.WriteString(" IS ") + if n.Comment != nil { + lex.EncodeSQLStringWithFlags(ctx.Buffer, *n.Comment, ctx.flags.EncodeFlags()) + } else { + ctx.WriteString("NULL") + } +} diff --git a/pkg/sql/sem/tree/stmt.go b/pkg/sql/sem/tree/stmt.go index 4c5702cd5e79..5114ceb80457 100644 --- a/pkg/sql/sem/tree/stmt.go +++ b/pkg/sql/sem/tree/stmt.go @@ -179,6 +179,12 @@ func (*AlterTable) StatementTag() string { return "ALTER TABLE" } func (*AlterTable) hiddenFromShowQueries() {} +// StatementType implements the Statement interface. +func (*CommentOnTable) StatementType() StatementType { return DDL } + +// StatementTag returns a short string identifying the type of statement. +func (*CommentOnTable) StatementTag() string { return "COMMENT ON TABLE" } + // StatementType implements the Statement interface. func (*AlterSequence) StatementType() StatementType { return DDL } @@ -797,6 +803,7 @@ func (n *AlterTableDropConstraint) String() string { return AsString(n) } func (n *AlterTableDropNotNull) String() string { return AsString(n) } func (n *AlterTableDropStored) String() string { return AsString(n) } func (n *AlterTableSetDefault) String() string { return AsString(n) } +func (n *CommentOnTable) String() string { return AsString(n) } func (n *AlterUserSetPassword) String() string { return AsString(n) } func (n *AlterSequence) String() string { return AsString(n) } func (n *Backup) String() string { return AsString(n) } diff --git a/pkg/sql/sqlbase/system.go b/pkg/sql/sqlbase/system.go index 8461896c935b..e5b63cec2caf 100644 --- a/pkg/sql/sqlbase/system.go +++ b/pkg/sql/sqlbase/system.go @@ -213,6 +213,16 @@ CREATE TABLE system.role_members ( INDEX ("role"), INDEX ("member") );` + + // comments stores comments(database, table, column...). + CommentsTableSchema = ` +CREATE TABLE system.comments ( + type INT NOT NULL, -- type of object, to distinguish between db and non-db objects + object_id INT NOT NULL, -- main object ID, this will be usually db/table desc ID + sub_id INT NOT NULL, -- sub-ID for columns inside table, 0 for pure table and other types + comment STRING NOT NULL, -- the comment + PRIMARY KEY (type, object_id, sub_id) +);` ) func pk(name string) IndexDescriptor { @@ -252,6 +262,7 @@ var SystemAllowedPrivileges = map[ID]privilege.List{ keys.TableStatisticsTableID: privilege.ReadWriteData, keys.LocationsTableID: privilege.ReadWriteData, keys.RoleMembersTableID: privilege.ReadWriteData, + keys.CommentsTableID: privilege.ReadWriteData, } // Helpers used to make some of the TableDescriptor literals below more concise. @@ -821,6 +832,38 @@ var ( FormatVersion: InterleavedFormatVersion, NextMutationID: 1, } + + // CommentsTable is the descriptor for the comments table. + CommentsTable = TableDescriptor{ + Name: "comments", + ID: keys.CommentsTableID, + ParentID: keys.SystemDatabaseID, + Version: 1, + Columns: []ColumnDescriptor{ + {Name: "type", ID: 1, Type: colTypeInt}, + {Name: "object_id", ID: 2, Type: colTypeInt}, + {Name: "sub_id", ID: 3, Type: colTypeInt}, + {Name: "comment", ID: 4, Type: colTypeString}, + }, + NextColumnID: 5, + Families: []ColumnFamilyDescriptor{ + {Name: "primary", ID: 0, ColumnNames: []string{"type", "object_id", "sub_id"}, ColumnIDs: []ColumnID{1, 2, 3}}, + {Name: "fam_4_comment", ID: 4, ColumnNames: []string{"comment"}, ColumnIDs: []ColumnID{4}, DefaultColumnID: 4}, + }, + NextFamilyID: 5, + PrimaryIndex: IndexDescriptor{ + Name: "primary", + ID: 1, + Unique: true, + ColumnNames: []string{"type", "object_id", "sub_id"}, + ColumnDirections: []IndexDescriptor_Direction{IndexDescriptor_ASC, IndexDescriptor_ASC, IndexDescriptor_ASC}, + ColumnIDs: []ColumnID{1, 2, 3}, + }, + NextIndexID: 2, + Privileges: NewCustomSuperuserPrivilegeDescriptor(SystemAllowedPrivileges[keys.CommentsTableID]), + FormatVersion: InterleavedFormatVersion, + NextMutationID: 1, + } ) // Create a kv pair for the zone config for the given key and config value. diff --git a/pkg/sql/tests/system_table_test.go b/pkg/sql/tests/system_table_test.go index 63646006e1e5..2462ce9e1eb8 100644 --- a/pkg/sql/tests/system_table_test.go +++ b/pkg/sql/tests/system_table_test.go @@ -120,6 +120,7 @@ func TestSystemTableLiterals(t *testing.T) { {keys.TableStatisticsTableID, sqlbase.TableStatisticsTableSchema, sqlbase.TableStatisticsTable}, {keys.LocationsTableID, sqlbase.LocationsTableSchema, sqlbase.LocationsTable}, {keys.RoleMembersTableID, sqlbase.RoleMembersTableSchema, sqlbase.RoleMembersTable}, + {keys.CommentsTableID, sqlbase.CommentsTableSchema, sqlbase.CommentsTable}, } { // Always create tables with "admin" privileges included, or CreateTestTableDescriptor fails. privs := sqlbase.NewCustomSuperuserPrivilegeDescriptor(sqlbase.SystemAllowedPrivileges[test.id]) diff --git a/pkg/sql/walk.go b/pkg/sql/walk.go index 91deb2d056a8..8d04af769a91 100644 --- a/pkg/sql/walk.go +++ b/pkg/sql/walk.go @@ -644,6 +644,7 @@ var planNodeNames = map[reflect.Type]string{ reflect.TypeOf(&alterSequenceNode{}): "alter sequence", reflect.TypeOf(&alterTableNode{}): "alter table", reflect.TypeOf(&alterUserSetPasswordNode{}): "alter user", + reflect.TypeOf(&commentOnTableNode{}): "comment on table", reflect.TypeOf(&cancelQueriesNode{}): "cancel queries", reflect.TypeOf(&cancelSessionsNode{}): "cancel sessions", reflect.TypeOf(&controlJobsNode{}): "control jobs", diff --git a/pkg/sqlmigrations/migrations.go b/pkg/sqlmigrations/migrations.go index 7f34f8a436cf..1048a8c9e3c4 100644 --- a/pkg/sqlmigrations/migrations.go +++ b/pkg/sqlmigrations/migrations.go @@ -190,6 +190,13 @@ var backwardCompatibleMigrations = []migrationDescriptor{ name: "add progress to system.jobs", workFn: addJobsProgress, }, + { + // Introduced in v2.1. + // TODO(hueypark): bake this migration into v2.2. + name: "create system.comment table", + workFn: createCommentTable, + newDescriptorIDs: staticIDs(keys.CommentsTableID), + }, } func staticIDs(ids ...sqlbase.ID) func(ctx context.Context, db db) ([]sqlbase.ID, error) { @@ -503,6 +510,10 @@ func createSystemTable(ctx context.Context, r runner, desc sqlbase.TableDescript return err } +func createCommentTable(ctx context.Context, r runner) error { + return createSystemTable(ctx, r, sqlbase.CommentsTable) +} + var reportingOptOut = envutil.EnvOrDefaultBool("COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING", false) func runStmtAsRootWithRetry(