Skip to content

Commit

Permalink
sql: support COMMENT ON TABLE
Browse files Browse the repository at this point in the history
Create system.comments table for comment. Each column description:
    - type: type of object, to distinguish between db, table, column and others(https://www.postgresql.org/docs/9.1/sql-comment.html)
    - object_id: object ID, this will be usually db/table desc ID
    - sub_id: sub ID for columns inside table, 0 for pure table
    - comment: the comment
And system.comments has public priviliege because this table used in SHOW TABLES.

See cockroachdb#19472

Release note (sql change): support COMMENT ON TABLE syntax
  • Loading branch information
hueypark committed Dec 2, 2018
1 parent 9f54fc1 commit afd5aab
Show file tree
Hide file tree
Showing 48 changed files with 1,090 additions and 481 deletions.
8 changes: 8 additions & 0 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ stmt ::=
'HELPTOKEN'
| preparable_stmt
| copy_from_stmt
| comment_stmt
| execute_stmt
| deallocate_stmt
| discard_stmt
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -196,6 +200,10 @@ opt_column_list ::=
'(' name_list ')'
|

comment_text ::=
'SCONST'
| 'NULL'

table_alias_name ::=
name

Expand Down
10 changes: 5 additions & 5 deletions pkg/ccl/importccl/import_stmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ COPY t (a, b, c) FROM stdin;
typ: "PGDUMP",
data: testPgdumpFk,
query: map[string][][]string{
`SHOW TABLES`: {{"cities"}, {"weather"}},
`SHOW TABLES`: {{"cities", "NULL"}, {"weather", "NULL"}},
`SELECT city FROM cities`: {{"Berkeley"}},
`SELECT city FROM weather`: {{"Berkeley"}},

Expand All @@ -478,7 +478,7 @@ COPY t (a, b, c) FROM stdin;
typ: "PGDUMP",
data: testPgdumpFkCircular,
query: map[string][][]string{
`SHOW TABLES`: {{"a"}, {"b"}},
`SHOW TABLES`: {{"a", "NULL"}, {"b", "NULL"}},
`SELECT i, k FROM a`: {{"2", "2"}},
`SELECT j FROM b`: {{"2"}},

Expand Down Expand Up @@ -528,7 +528,7 @@ COPY t (a, b, c) FROM stdin;
data: testPgdumpFk,
with: `WITH skip_foreign_keys`,
query: map[string][][]string{
`SHOW TABLES`: {{"cities"}, {"weather"}},
`SHOW TABLES`: {{"cities", "NULL"}, {"weather", "NULL"}},
// Verify the constraint is skipped.
`SELECT dependson_name FROM crdb_internal.backward_dependencies`: {},
`SHOW CONSTRAINTS FROM weather`: {},
Expand All @@ -546,7 +546,7 @@ COPY t (a, b, c) FROM stdin;
data: testPgdumpFk,
with: `WITH skip_foreign_keys`,
query: map[string][][]string{
`SHOW TABLES`: {{"weather"}},
`SHOW TABLES`: {{"weather", "NULL"}},
},
},
{
Expand Down Expand Up @@ -602,7 +602,7 @@ COPY t (a, b, c) FROM stdin;
CREATE TABLE t (i INT);
`,
query: map[string][][]string{
`SHOW TABLES`: {{"t"}},
`SHOW TABLES`: {{"t", "NULL"}},
},
},
{
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/logictestccl/testdata/logic_test/role
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions pkg/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions pkg/keys/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
2 changes: 1 addition & 1 deletion pkg/server/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func (s *adminServer) DatabaseDetails(
{
const nameCol = "table_name"
scanner := makeResultScanner(cols)
if a, e := len(cols), 1; a != e {
if a, e := len(cols), 2; a != e {
return nil, s.serverErrorf("show tables columns mismatch: %d != expected %d", a, e)
}
for _, row := range rows {
Expand Down
97 changes: 97 additions & 0 deletions pkg/sql/comment_on_table.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
// 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 tableDesc == nil {
return newZeroNode(nil /* columns */), nil
}

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) {}
114 changes: 114 additions & 0 deletions pkg/sql/comment_on_table_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
// 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"
"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 sql.NullString
}{
{
`COMMENT ON TABLE t IS 'foo'`,
`SELECT obj_description('t'::regclass)`,
sql.NullString{String: `foo`, Valid: true},
},
{
`TRUNCATE t`,
`SELECT obj_description('t'::regclass)`,
sql.NullString{String: `foo`, Valid: true},
},
{
`COMMENT ON TABLE t IS NULL`,
`SELECT obj_description('t'::regclass)`,
sql.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 sql.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 != sql.ErrNoRows {
if err != nil {
t.Fatal(err)
}

t.Fatal("dropped comment remain comment")
}
}
22 changes: 21 additions & 1 deletion pkg/sql/drop_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,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"
Expand Down Expand Up @@ -314,7 +315,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
}

Expand Down Expand Up @@ -476,3 +482,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
}
2 changes: 2 additions & 0 deletions pkg/sql/event_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/expand_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ func doExpandPlan(
case *alterTableNode:
case *alterSequenceNode:
case *alterUserSetPasswordNode:
case *commentOnTableNode:
case *renameColumnNode:
case *renameDatabaseNode:
case *renameIndexNode:
Expand Down Expand Up @@ -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:
Expand Down
Loading

0 comments on commit afd5aab

Please sign in to comment.