diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf index a93863fec93f..8a5dc5ffddb4 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 @@ -198,6 +202,10 @@ opt_column_list ::= '(' name_list ')' | +comment_text ::= + 'SCONST' + | 'NULL' + table_alias_name ::= name @@ -488,9 +496,9 @@ show_stats_stmt ::= 'SHOW' 'STATISTICS' 'FOR' 'TABLE' table_name show_tables_stmt ::= - 'SHOW' 'TABLES' 'FROM' name '.' name - | 'SHOW' 'TABLES' 'FROM' name - | 'SHOW' 'TABLES' + 'SHOW' 'TABLES' 'FROM' name '.' name with_comment + | 'SHOW' 'TABLES' 'FROM' name with_comment + | 'SHOW' 'TABLES' with_comment show_trace_stmt ::= 'SHOW' opt_compact 'TRACE' 'FOR' 'SESSION' @@ -1097,6 +1105,10 @@ table_name_with_index ::= table_name '@' index_name | table_name +with_comment ::= + 'WITH' 'COMMENT' + | + opt_compact ::= 'COMPACT' | diff --git a/pkg/ccl/logictestccl/testdata/logic_test/role b/pkg/ccl/logictestccl/testdata/logic_test/role index 5691d7b80d48..69400b4441de 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/role +++ b/pkg/ccl/logictestccl/testdata/logic_test/role @@ -718,10 +718,10 @@ CREATE USER public statement error role name "public" is reserved CREATE ROLE public -statement error cannot drop special role public +statement error cannot drop user or role public: grants still exist on system.public.comments DROP USER public -statement error cannot drop special role public +statement error cannot drop user or role public: grants still exist on system.public.comments DROP ROLE public statement error role public does not exist diff --git a/pkg/cli/cli_test.go b/pkg/cli/cli_test.go index a9371279bc45..f7fbce55c481 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..4cf2ab316a3b 100644 --- a/pkg/keys/constants.go +++ b/pkg/keys/constants.go @@ -328,4 +328,10 @@ const ( LocationsTableID = 21 LivenessRangesID = 22 RoleMembersTableID = 23 + CommentsTableID = 24 + + // CommentType is type for system.comments + // DatabaseCommentType = 0 + TableCommentType = 1 + // ColumnCommentType = 2 ) diff --git a/pkg/sql/comment_on_table.go b/pkg/sql/comment_on_table.go new file mode 100644 index 000000000000..0684857bc0c9 --- /dev/null +++ b/pkg/sql/comment_on_table.go @@ -0,0 +1,94 @@ +// 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" +) + +type commentOnTableNode struct { + n *tree.CommentOnTable + tableDesc *MutableTableDescriptor +} + +// 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 err := p.CheckPrivilege(ctx, tableDesc, privilege.CREATE); err != nil { + return nil, err + } + + return &commentOnTableNode{n: n, tableDesc: tableDesc}, nil +} + +func (n *commentOnTableNode) startExec(params runParams) error { + 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, + n.tableDesc.ID, + *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, + n.tableDesc.ID) + 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/comment_on_table_test.go b/pkg/sql/comment_on_table_test.go new file mode 100644 index 000000000000..575373a3478a --- /dev/null +++ b/pkg/sql/comment_on_table_test.go @@ -0,0 +1,113 @@ +// 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_test + +import ( + "context" + gosql "database/sql" + "testing" + + "github.com/cockroachdb/cockroach/pkg/sql/tests" + "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" +) + +func TestCommentOnTable(t *testing.T) { + defer leaktest.AfterTest(t)() + + params, _ := tests.CreateTestServerParams() + s, db, _ := serverutils.StartServer(t, params) + defer s.Stopper().Stop(context.TODO()) + + if _, err := db.Exec(` + CREATE DATABASE d; + SET DATABASE = d; + CREATE TABLE t (i INT ); + `); err != nil { + t.Fatal(err) + } + + testCases := []struct { + exec string + query string + expect gosql.NullString + }{ + { + `COMMENT ON TABLE t IS 'foo'`, + `SELECT obj_description('t'::regclass)`, + gosql.NullString{String: `foo`, Valid: true}, + }, + { + `TRUNCATE t`, + `SELECT obj_description('t'::regclass)`, + gosql.NullString{String: `foo`, Valid: true}, + }, + { + `COMMENT ON TABLE t IS NULL`, + `SELECT obj_description('t'::regclass)`, + gosql.NullString{Valid: false}, + }, + } + + for _, tc := range testCases { + if _, err := db.Exec(tc.exec); err != nil { + t.Fatal(err) + } + + row := db.QueryRow(tc.query) + var comment gosql.NullString + if err := row.Scan(&comment); err != nil { + t.Fatal(err) + } + if tc.expect != comment { + t.Fatalf("expected comment %v, got %v", tc.expect, comment) + } + } +} + +func TestCommentOnTableWhenDrop(t *testing.T) { + defer leaktest.AfterTest(t)() + + params, _ := tests.CreateTestServerParams() + s, db, _ := serverutils.StartServer(t, params) + defer s.Stopper().Stop(context.TODO()) + + if _, err := db.Exec(` + CREATE DATABASE d; + SET DATABASE = d; + CREATE TABLE t (i INT ); + `); err != nil { + t.Fatal(err) + } + + if _, err := db.Exec(`COMMENT ON TABLE t IS 'foo'`); err != nil { + t.Fatal(err) + } + + if _, err := db.Exec(`DROP TABLE t`); err != nil { + t.Fatal(err) + } + + row := db.QueryRow(`SELECT comment FROM system.comments LIMIT 1`) + var comment string + err := row.Scan(&comment) + if err != gosql.ErrNoRows { + if err != nil { + t.Fatal(err) + } + + t.Fatal("dropped comment remain comment") + } +} diff --git a/pkg/sql/drop_table.go b/pkg/sql/drop_table.go index ab29b08b5b1c..7c8edf810d96 100644 --- a/pkg/sql/drop_table.go +++ b/pkg/sql/drop_table.go @@ -20,6 +20,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/jobs" "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" + "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/privilege" @@ -313,7 +314,12 @@ func (p *planner) dropTableImpl( droppedViews = append(droppedViews, viewDesc.Name) } - err := p.initiateDropTable(ctx, tableDesc, true /* drain name */) + err := p.removeComment(ctx, tableDesc) + if err != nil { + return droppedViews, err + } + + err = p.initiateDropTable(ctx, tableDesc, true /* drain name */) return droppedViews, err } @@ -475,3 +481,17 @@ func removeMatchingReferences( } return updatedRefs } + +func (p *planner) removeComment( + ctx context.Context, tableDesc *sqlbase.MutableTableDescriptor, +) error { + _, err := p.ExtendedEvalContext().ExecCfg.InternalExecutor.Exec( + ctx, + "delete-comment", + p.txn, + "DELETE FROM system.comments WHERE type=$1 AND object_id=$2 AND sub_id=0", + keys.TableCommentType, + tableDesc.ID) + + return err +} diff --git a/pkg/sql/event_log.go b/pkg/sql/event_log.go index fe39b9555ca6..61910b8703f9 100644 --- a/pkg/sql/event_log.go +++ b/pkg/sql/event_log.go @@ -43,6 +43,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 a0c6f6a8acc5..93adb0c94846 100644 --- a/pkg/sql/expand_plan.go +++ b/pkg/sql/expand_plan.go @@ -350,6 +350,7 @@ func doExpandPlan( case *alterTableNode: case *alterSequenceNode: case *alterUserSetPasswordNode: + case *commentOnTableNode: case *renameColumnNode: case *renameDatabaseNode: case *renameIndexNode: @@ -860,6 +861,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..f63ba3ac3755 100644 --- a/pkg/sql/logictest/testdata/logic_test/grant_table +++ b/pkg/sql/logictest/testdata/logic_test/grant_table @@ -160,6 +160,21 @@ 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 public DELETE +system public comments public GRANT +system public comments public INSERT +system public comments public SELECT +system public comments public 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 +330,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..c995d82c219a 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,21 @@ 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 public system public comments DELETE NULL NULL +NULL public system public comments GRANT NULL NULL +NULL public system public comments INSERT NULL NULL +NULL public system public comments SELECT NULL NULL +NULL public 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 +1506,21 @@ 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 public system public comments DELETE NULL NULL +NULL public system public comments GRANT NULL NULL +NULL public system public comments INSERT NULL NULL +NULL public system public comments SELECT NULL NULL +NULL public 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 ef615346c845..437829cf61d4 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/role b/pkg/sql/logictest/testdata/logic_test/role index ffb96db38fe9..54d52fbdad54 100644 --- a/pkg/sql/logictest/testdata/logic_test/role +++ b/pkg/sql/logictest/testdata/logic_test/role @@ -45,7 +45,7 @@ role_name member is_admin statement error role name "public" is reserved CREATE USER public -statement error cannot drop special role public +statement error cannot drop user or role public: grants still exist on system.public.comments DROP USER public statement ok diff --git a/pkg/sql/logictest/testdata/logic_test/show_source b/pkg/sql/logictest/testdata/logic_test/show_source index 7ebd51edca43..d7bcd08751fd 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 @@ -157,6 +158,26 @@ users web_sessions zones +query TT colnames +SELECT * FROM [SHOW TABLES FROM system WITH COMMENT] +---- +table_name comment +comments NULL +descriptor NULL +eventlog NULL +jobs NULL +lease NULL +locations NULL +namespace NULL +rangelog NULL +role_members NULL +settings NULL +table_statistics NULL +ui NULL +users NULL +web_sessions NULL +zones NULL + query ITTT colnames SELECT node_id, user_name, application_name, active_queries FROM [SHOW SESSIONS] diff --git a/pkg/sql/logictest/testdata/logic_test/system b/pkg/sql/logictest/testdata/logic_test/system index f3e0d796abb3..03d6c5152c59 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,134 +191,149 @@ system public root SELECT query TTTTT SHOW GRANTS ON system.* ---- -system public descriptor admin GRANT -system public descriptor admin SELECT -system public descriptor root GRANT -system public descriptor root SELECT -system public eventlog admin DELETE -system public eventlog admin GRANT -system public eventlog admin INSERT -system public eventlog admin SELECT -system public eventlog admin UPDATE -system public eventlog root DELETE -system public eventlog root GRANT -system public eventlog root INSERT -system public eventlog root SELECT -system public eventlog root UPDATE -system public jobs admin DELETE -system public jobs admin GRANT -system public jobs admin INSERT -system public jobs admin SELECT -system public jobs admin UPDATE -system public jobs root DELETE -system public jobs root GRANT -system public jobs root INSERT -system public jobs root SELECT -system public jobs root UPDATE -system public lease admin DELETE -system public lease admin GRANT -system public lease admin INSERT -system public lease admin SELECT -system public lease admin UPDATE -system public lease root DELETE -system public lease root GRANT -system public lease root INSERT -system public lease root SELECT -system public lease root UPDATE -system public locations admin DELETE -system public locations admin GRANT -system public locations admin INSERT -system public locations admin SELECT -system public locations admin UPDATE -system public locations root DELETE -system public locations root GRANT -system public locations root INSERT -system public locations root SELECT -system public locations root UPDATE -system public namespace admin GRANT -system public namespace admin SELECT -system public namespace root GRANT -system public namespace root SELECT -system public rangelog admin DELETE -system public rangelog admin GRANT -system public rangelog admin INSERT -system public rangelog admin SELECT -system public rangelog admin UPDATE -system public rangelog root DELETE -system public rangelog root GRANT -system public rangelog root INSERT -system public rangelog root SELECT -system public rangelog root UPDATE -system public role_members admin DELETE -system public role_members admin GRANT -system public role_members admin INSERT -system public role_members admin SELECT -system public role_members admin UPDATE -system public role_members root DELETE -system public role_members root GRANT -system public role_members root INSERT -system public role_members root SELECT -system public role_members root UPDATE -system public settings admin DELETE -system public settings admin GRANT -system public settings admin INSERT -system public settings admin SELECT -system public settings admin UPDATE -system public settings root DELETE -system public settings root GRANT -system public settings root INSERT -system public settings root SELECT -system public settings root UPDATE -system public table_statistics admin DELETE -system public table_statistics admin GRANT -system public table_statistics admin INSERT -system public table_statistics admin SELECT -system public table_statistics admin UPDATE -system public table_statistics root DELETE -system public table_statistics root GRANT -system public table_statistics root INSERT -system public table_statistics root SELECT -system public table_statistics root UPDATE -system public ui admin DELETE -system public ui admin GRANT -system public ui admin INSERT -system public ui admin SELECT -system public ui admin UPDATE -system public ui root DELETE -system public ui root GRANT -system public ui root INSERT -system public ui root SELECT -system public ui root UPDATE -system public users admin DELETE -system public users admin GRANT -system public users admin INSERT -system public users admin SELECT -system public users admin UPDATE -system public users root DELETE -system public users root GRANT -system public users root INSERT -system public users root SELECT -system public users root UPDATE -system public web_sessions admin DELETE -system public web_sessions admin GRANT -system public web_sessions admin INSERT -system public web_sessions admin SELECT -system public web_sessions admin UPDATE -system public web_sessions root DELETE -system public web_sessions root GRANT -system public web_sessions root INSERT -system public web_sessions root SELECT -system public web_sessions root UPDATE -system public zones admin DELETE -system public zones admin GRANT -system public zones admin INSERT -system public zones admin SELECT -system public zones admin UPDATE -system public zones root DELETE -system public zones root GRANT -system public zones root INSERT -system public zones root SELECT -system public zones root UPDATE +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 public DELETE +system public comments public GRANT +system public comments public INSERT +system public comments public SELECT +system public comments public 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 +system public descriptor root SELECT +system public eventlog admin DELETE +system public eventlog admin GRANT +system public eventlog admin INSERT +system public eventlog admin SELECT +system public eventlog admin UPDATE +system public eventlog root DELETE +system public eventlog root GRANT +system public eventlog root INSERT +system public eventlog root SELECT +system public eventlog root UPDATE +system public jobs admin DELETE +system public jobs admin GRANT +system public jobs admin INSERT +system public jobs admin SELECT +system public jobs admin UPDATE +system public jobs root DELETE +system public jobs root GRANT +system public jobs root INSERT +system public jobs root SELECT +system public jobs root UPDATE +system public lease admin DELETE +system public lease admin GRANT +system public lease admin INSERT +system public lease admin SELECT +system public lease admin UPDATE +system public lease root DELETE +system public lease root GRANT +system public lease root INSERT +system public lease root SELECT +system public lease root UPDATE +system public locations admin DELETE +system public locations admin GRANT +system public locations admin INSERT +system public locations admin SELECT +system public locations admin UPDATE +system public locations root DELETE +system public locations root GRANT +system public locations root INSERT +system public locations root SELECT +system public locations root UPDATE +system public namespace admin GRANT +system public namespace admin SELECT +system public namespace root GRANT +system public namespace root SELECT +system public rangelog admin DELETE +system public rangelog admin GRANT +system public rangelog admin INSERT +system public rangelog admin SELECT +system public rangelog admin UPDATE +system public rangelog root DELETE +system public rangelog root GRANT +system public rangelog root INSERT +system public rangelog root SELECT +system public rangelog root UPDATE +system public role_members admin DELETE +system public role_members admin GRANT +system public role_members admin INSERT +system public role_members admin SELECT +system public role_members admin UPDATE +system public role_members root DELETE +system public role_members root GRANT +system public role_members root INSERT +system public role_members root SELECT +system public role_members root UPDATE +system public settings admin DELETE +system public settings admin GRANT +system public settings admin INSERT +system public settings admin SELECT +system public settings admin UPDATE +system public settings root DELETE +system public settings root GRANT +system public settings root INSERT +system public settings root SELECT +system public settings root UPDATE +system public table_statistics admin DELETE +system public table_statistics admin GRANT +system public table_statistics admin INSERT +system public table_statistics admin SELECT +system public table_statistics admin UPDATE +system public table_statistics root DELETE +system public table_statistics root GRANT +system public table_statistics root INSERT +system public table_statistics root SELECT +system public table_statistics root UPDATE +system public ui admin DELETE +system public ui admin GRANT +system public ui admin INSERT +system public ui admin SELECT +system public ui admin UPDATE +system public ui root DELETE +system public ui root GRANT +system public ui root INSERT +system public ui root SELECT +system public ui root UPDATE +system public users admin DELETE +system public users admin GRANT +system public users admin INSERT +system public users admin SELECT +system public users admin UPDATE +system public users root DELETE +system public users root GRANT +system public users root INSERT +system public users root SELECT +system public users root UPDATE +system public web_sessions admin DELETE +system public web_sessions admin GRANT +system public web_sessions admin INSERT +system public web_sessions admin SELECT +system public web_sessions admin UPDATE +system public web_sessions root DELETE +system public web_sessions root GRANT +system public web_sessions root INSERT +system public web_sessions root SELECT +system public web_sessions root UPDATE +system public zones admin DELETE +system public zones admin GRANT +system public zones admin INSERT +system public zones admin SELECT +system public zones admin UPDATE +system public zones root DELETE +system public zones root GRANT +system public zones root INSERT +system public zones root SELECT +system public zones root UPDATE statement error user root does not have DROP privilege on database system ALTER DATABASE system RENAME TO not_system diff --git a/pkg/sql/logictest/testdata/logic_test/table b/pkg/sql/logictest/testdata/logic_test/table index 412b68e7902e..8e26016add72 100644 --- a/pkg/sql/logictest/testdata/logic_test/table +++ b/pkg/sql/logictest/testdata/logic_test/table @@ -39,6 +39,9 @@ CREATE TABLE dup_unique (a int, unique (a,a)) statement ok CREATE TABLE IF NOT EXISTS a (id INT PRIMARY KEY) +statement ok +COMMENT ON TABLE a IS 'a_comment' + query T colnames SHOW TABLES FROM test ---- @@ -114,15 +117,15 @@ a INT false NULL · {pr b INT false NULL · {primary} false c INT false NULL · {primary} false -query T -SHOW TABLES FROM test +query TT +SHOW TABLES FROM test WITH COMMENT ---- -a -b -c -d -e -f +a a_comment +b NULL +c NULL +d NULL +e NULL +f NULL statement ok SET DATABASE = "" diff --git a/pkg/sql/logictest/testdata/planner_test/explain b/pkg/sql/logictest/testdata/planner_test/explain index 2ac8e482b730..798ff6d18089 100644 --- a/pkg/sql/logictest/testdata/planner_test/explain +++ b/pkg/sql/logictest/testdata/planner_test/explain @@ -156,13 +156,26 @@ distinct · · query TTT EXPLAIN SHOW TABLES ---- -sort · · - │ order +table_schema,+table_name - └── render · · - └── filter · · - │ filter table_schema = 'public' - └── values · · -· size 6 columns, 92 rows +sort · · + │ order +table_schema,+table_name + └── render · · + └── join · · + │ type left outer + │ equality (table_id) = (object_id) + ├── filter · · + │ │ filter ((state = 'PUBLIC') OR (state IS NULL)) AND ((database_name = 'test') OR (database_name IS NULL)) + │ └── join · · + │ │ type left outer + │ │ equality (table_name, table_catalog) = (name, database_name) + │ ├── filter · · + │ │ │ filter table_schema = 'public' + │ │ └── values · · + │ │ size 6 columns, 92 rows + │ └── values · · + │ size 13 columns, 16 rows + └── scan · · +· table comments@primary +· spans ALL query TTT EXPLAIN SHOW DATABASE @@ -249,8 +262,7 @@ sort · · └── filter · · │ filter (table_catalog, table_schema, table_name) IN (('test', 'public', 'foo'),) └── values · · -· size 8 columns, 585 rows - +· size 8 columns, 600 rows query TTT EXPLAIN SHOW INDEX FROM foo diff --git a/pkg/sql/opt/exec/execbuilder/testdata/explain b/pkg/sql/opt/exec/execbuilder/testdata/explain index 9369ae18023c..a109b3c90fe8 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/explain +++ b/pkg/sql/opt/exec/execbuilder/testdata/explain @@ -147,13 +147,26 @@ distinct · · query TTT EXPLAIN SHOW TABLES ---- -sort · · - │ order +table_schema,+table_name - └── render · · - └── filter · · - │ filter table_schema = 'public' - └── values · · -· size 6 columns, 92 rows +sort · · + │ order +table_schema,+table_name + └── render · · + └── join · · + │ type left outer + │ equality (table_id) = (object_id) + ├── filter · · + │ │ filter ((state = 'PUBLIC') OR (state IS NULL)) AND ((database_name = 'test') OR (database_name IS NULL)) + │ └── join · · + │ │ type left outer + │ │ equality (table_name, table_catalog) = (name, database_name) + │ ├── filter · · + │ │ │ filter table_schema = 'public' + │ │ └── values · · + │ │ size 6 columns, 92 rows + │ └── values · · + │ size 13 columns, 16 rows + └── scan · · +· table comments@primary +· spans ALL query TTT EXPLAIN SHOW DATABASE @@ -240,8 +253,7 @@ sort · · └── filter · · │ filter (table_catalog, table_schema, table_name) IN (('test', 'public', 'foo'),) └── values · · -· size 8 columns, 585 rows - +· size 8 columns, 600 rows query TTT EXPLAIN SHOW INDEX FROM foo diff --git a/pkg/sql/opt_filters.go b/pkg/sql/opt_filters.go index 34382d90279b..35a664df0395 100644 --- a/pkg/sql/opt_filters.go +++ b/pkg/sql/opt_filters.go @@ -371,6 +371,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 601aad9945d5..3a8b81a564f2 100644 --- a/pkg/sql/opt_limits.go +++ b/pkg/sql/opt_limits.go @@ -216,6 +216,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 0e97db35efb5..61c46f1a4ff9 100644 --- a/pkg/sql/opt_needed.go +++ b/pkg/sql/opt_needed.go @@ -264,6 +264,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 ab13f0dc9727..ef55e7993562 100644 --- a/pkg/sql/parser/parse_test.go +++ b/pkg/sql/parser/parse_test.go @@ -371,9 +371,12 @@ func TestParse(t *testing.T) { {`EXPLAIN SHOW SCHEMAS`}, {`SHOW SCHEMAS FROM a`}, {`SHOW TABLES`}, + {`SHOW TABLES WITH COMMENT`}, {`EXPLAIN SHOW TABLES`}, {`SHOW TABLES FROM a`}, + {`SHOW TABLES FROM a WITH COMMENT`}, {`SHOW TABLES FROM a.b`}, + {`SHOW TABLES FROM a.b WITH COMMENT`}, {`SHOW COLUMNS FROM a`}, {`EXPLAIN SHOW COLUMNS FROM a`}, {`SHOW COLUMNS FROM a.b.c`}, @@ -1127,6 +1130,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 +2581,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 d85de93fea34..b9373adecbaf 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 @@ -819,6 +819,7 @@ func newNameFromStr(s string) *tree.Name { %type sequence_option_elem %type all_or_distinct +%type with_comment %type join_outer %type join_qual %type join_type @@ -2008,7 +2009,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 { @@ -2020,8 +2026,15 @@ comment_stmt: } comment_text: - SCONST { $$ = $1 } - | NULL { $$ = "" } + SCONST + { + $$.val = &$1 + } +| NULL + { + var str *string + $$.val = str + } // %Help: CREATE // %Category: Group @@ -3368,30 +3381,36 @@ show_sessions_stmt: // %Text: SHOW TABLES [FROM [ . ] ] // %SeeAlso: WEBDOCS/show-tables.html show_tables_stmt: - SHOW TABLES FROM name '.' name + SHOW TABLES FROM name '.' name with_comment { $$.val = &tree.ShowTables{TableNamePrefix:tree.TableNamePrefix{ CatalogName: tree.Name($4), ExplicitCatalog: true, SchemaName: tree.Name($6), ExplicitSchema: true, - }} + }, + WithComment: $7.bool()} } -| SHOW TABLES FROM name +| SHOW TABLES FROM name with_comment { $$.val = &tree.ShowTables{TableNamePrefix:tree.TableNamePrefix{ // Note: the schema name may be interpreted as database name, // see name_resolution.go. SchemaName: tree.Name($4), ExplicitSchema: true, - }} + }, + WithComment: $5.bool()} } -| SHOW TABLES +| SHOW TABLES with_comment { - $$.val = &tree.ShowTables{} + $$.val = &tree.ShowTables{WithComment: $3.bool()} } | SHOW TABLES error // SHOW HELP: SHOW TABLES +with_comment: + WITH COMMENT { $$.val = true } +| /* EMPTY */ { $$.val = false } + // %Help: SHOW SCHEMAS - list schemas // %Category: DDL // %Text: SHOW SCHEMAS [FROM ] diff --git a/pkg/sql/pg_catalog.go b/pkg/sql/pg_catalog.go index b13c8f331995..6a066dbef65c 100644 --- a/pkg/sql/pg_catalog.go +++ b/pkg/sql/pg_catalog.go @@ -928,10 +928,49 @@ CREATE TABLE pg_catalog.pg_description ( classoid OID, objsubid INT, description STRING -)`, - populate: func(_ context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error { - // Comments on database objects are not currently supported. - return nil +); +`, + 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 + } + + commentMap := make(map[tree.DInt]tree.Datums) + for _, comment := range comments { + id := *comment[0].(*tree.DInt) + commentMap[id] = comment + } + + h := makeOidHasher() + return forEachTableDescWithTableLookup( + ctx, + p, + dbContext, + hideVirtual, + func( + db *sqlbase.DatabaseDescriptor, + scName string, + table *sqlbase.TableDescriptor, + tableLookup tableLookupFn) error { + if comment, ok := commentMap[tree.DInt(table.ID)]; ok { + return addRow( + h.TableOid(db, scName, table), + oidZero, + comment[1], + comment[2]) + } + + return nil + }) }, } diff --git a/pkg/sql/pgwire/pgwire_test.go b/pkg/sql/pgwire/pgwire_test.go index be8ce22968dd..2b91b0ba3a40 100644 --- a/pkg/sql/pgwire/pgwire_test.go +++ b/pkg/sql/pgwire/pgwire_test.go @@ -733,7 +733,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 f3d3a9b40b95..700159e804f2 100644 --- a/pkg/sql/plan.go +++ b/pkg/sql/plan.go @@ -793,6 +793,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 909bf9116ced..a0e19add5fbb 100644 --- a/pkg/sql/sem/builtins/pg_builtins.go +++ b/pkg/sql/sem/builtins/pg_builtins.go @@ -722,8 +722,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 description FROM pg_catalog.pg_description WHERE objoid=$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/show.go b/pkg/sql/sem/tree/show.go index fe4b123dbc3d..d9a81bb129f3 100644 --- a/pkg/sql/sem/tree/show.go +++ b/pkg/sql/sem/tree/show.go @@ -197,6 +197,7 @@ func (node *ShowSchemas) Format(ctx *FmtCtx) { // ShowTables represents a SHOW TABLES statement. type ShowTables struct { TableNamePrefix + WithComment bool } // Format implements the NodeFormatter interface. @@ -206,6 +207,10 @@ func (node *ShowTables) Format(ctx *FmtCtx) { ctx.WriteString(" FROM ") ctx.FormatNode(&node.TableNamePrefix) } + + if node.WithComment { + ctx.WriteString(" WITH COMMENT") + } } // ShowConstraints represents a SHOW CONSTRAINTS statement. diff --git a/pkg/sql/sem/tree/stmt.go b/pkg/sql/sem/tree/stmt.go index 1e58c378583f..260fe87d7e66 100644 --- a/pkg/sql/sem/tree/stmt.go +++ b/pkg/sql/sem/tree/stmt.go @@ -177,6 +177,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 } @@ -795,6 +801,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/show_tables.go b/pkg/sql/show_tables.go index 642e75eb7ceb..a1f8b0aa4035 100644 --- a/pkg/sql/show_tables.go +++ b/pkg/sql/show_tables.go @@ -40,13 +40,49 @@ func (p *planner) ShowTables(ctx context.Context, n *tree.ShowTables) (planNode, return nil, sqlbase.NewInvalidWildcardError(tree.ErrString(&n.TableNamePrefix)) } - const getTablesQuery = ` + var query string + if n.WithComment { + // TODO(knz): this 3-way join is painful and would really benefit + // from vtables having a real table ID (#32963) so that we don't + // have to use `information_schema.tables` and simply join + // `crdb_internal.tables` with `system.comments` instead. + const getTablesQuery = ` +SELECT + i.table_name, c.comment +FROM + %[1]s.information_schema.tables AS i + LEFT JOIN crdb_internal.tables AS t + ON + i.table_name = t.name + AND i.table_catalog = t.database_name + LEFT JOIN system.comments AS c + ON t.table_id = c.object_id +WHERE + table_schema = %[2]s + AND (t.state = %[3]s OR t.state IS NULL) + AND (t.database_name = %[4]s OR t.database_name IS NULL) +ORDER BY + table_schema, table_name` + + query = fmt.Sprintf( + getTablesQuery, + &n.CatalogName, + lex.EscapeSQLString(n.Schema()), + lex.EscapeSQLString(sqlbase.TableDescriptor_PUBLIC.String()), + lex.EscapeSQLString(n.CatalogName.Normalize())) + + } else { + const getTablesQuery = ` SELECT table_name FROM %[1]s.information_schema.tables WHERE table_schema = %[2]s ORDER BY table_schema, table_name` + query = fmt.Sprintf(getTablesQuery, + &n.CatalogName, lex.EscapeSQLString(n.Schema())) + } + return p.delegateQuery(ctx, "SHOW TABLES", - fmt.Sprintf(getTablesQuery, &n.CatalogName, lex.EscapeSQLString(n.Schema())), + query, func(_ context.Context) error { return nil }, nil) } diff --git a/pkg/sql/sqlbase/system.go b/pkg/sql/sqlbase/system.go index 8461896c935b..f3585e0ed370 100644 --- a/pkg/sql/sqlbase/system.go +++ b/pkg/sql/sqlbase/system.go @@ -20,6 +20,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/config" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/sql/privilege" ) @@ -213,6 +214,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, table, column and others + object_id INT NOT NULL, -- 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 + comment STRING NOT NULL, -- the comment + PRIMARY KEY (type, object_id, sub_id) +);` ) func pk(name string) IndexDescriptor { @@ -252,6 +263,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 +833,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: newCommentPrivilegeDescriptor(SystemAllowedPrivileges[keys.CommentsTableID]), + FormatVersion: InterleavedFormatVersion, + NextMutationID: 1, + } ) // Create a kv pair for the zone config for the given key and config value. @@ -904,3 +948,23 @@ func IsSystemConfigID(id ID) bool { func IsReservedID(id ID) bool { return id > 0 && id <= keys.MaxReservedDescID } + +// newCommentPrivilegeDescriptor returns a privilege descriptor for comment table +func newCommentPrivilegeDescriptor(priv privilege.List) *PrivilegeDescriptor { + return &PrivilegeDescriptor{ + Users: []UserPrivileges{ + { + User: AdminRole, + Privileges: priv.ToBitField(), + }, + { + User: PublicRole, + Privileges: priv.ToBitField(), + }, + { + User: security.RootUser, + Privileges: priv.ToBitField(), + }, + }, + } +} diff --git a/pkg/sql/tests/system_table_test.go b/pkg/sql/tests/system_table_test.go index dfe0a0e86cf1..034e8a00a9db 100644 --- a/pkg/sql/tests/system_table_test.go +++ b/pkg/sql/tests/system_table_test.go @@ -119,15 +119,15 @@ 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]) + privs := *test.pkg.Privileges gen, err := sql.CreateTestTableDescriptor( context.TODO(), keys.SystemDatabaseID, test.id, test.schema, - privs, + &privs, ) if err != nil { t.Fatalf("test: %+v, err: %v", test, err) diff --git a/pkg/sql/truncate.go b/pkg/sql/truncate.go index c323080856ad..360acd7c3fa4 100644 --- a/pkg/sql/truncate.go +++ b/pkg/sql/truncate.go @@ -19,6 +19,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/internal/client" "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" + "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/sql/privilege" "github.com/cockroachdb/cockroach/pkg/sql/row" @@ -273,6 +274,11 @@ func (p *planner) truncateTable( return err } + // Reassign comment. + if err := reassignComment(ctx, p, id, newID); err != nil { + return err + } + // Copy the zone config. b = &client.Batch{} b.Get(zoneKey) @@ -376,6 +382,42 @@ func reassignReferencedTables( return changed, nil } +// reassignComment reassign comment on table +func reassignComment(ctx context.Context, p *planner, oldID, newID sqlbase.ID) error { + comment, err := p.ExtendedEvalContext().ExecCfg.InternalExecutor.QueryRow( + ctx, + "select-comment", + p.txn, + `SELECT comment FROM system.comments WHERE object_id=$1`, + oldID) + if err != nil { + return err + } + + if comment != nil { + _, err = p.ExtendedEvalContext().ExecCfg.InternalExecutor.Exec( + ctx, + "upsert-comment", + p.txn, + "UPSERT INTO system.comments VALUES ($1, $2, 0, $3)", + keys.TableCommentType, + newID, + comment[0]) + if err != nil { + return err + } + } + + _, err = p.ExtendedEvalContext().ExecCfg.InternalExecutor.Exec( + ctx, + "delete-comment", + p.txn, + "DELETE FROM system.comments WHERE type=$1 AND object_id=$2 AND sub_id=0", + keys.TableCommentType, + oldID) + return err +} + // truncateTableInChunks truncates the data of a table in chunks. It deletes a // range of data for the table, which includes the PK and all indexes. // The table has already been marked for deletion and has been purged from the diff --git a/pkg/sql/walk.go b/pkg/sql/walk.go index d80827c1cdcf..4c2c4b179d4b 100644 --- a/pkg/sql/walk.go +++ b/pkg/sql/walk.go @@ -666,6 +666,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 f183ce7f4182..6fd29a6997d9 100644 --- a/pkg/sqlmigrations/migrations.go +++ b/pkg/sqlmigrations/migrations.go @@ -189,6 +189,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) { @@ -502,6 +509,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(