From 740f73dea8558977b4e957b309f6dca7f6f99a5e Mon Sep 17 00:00:00 2001 From: angelapwen Date: Wed, 21 Oct 2020 10:54:44 -0400 Subject: [PATCH] sql: allow parsing of DROP OWNED BY Add parsing and telemetry support for DROP OWNED BY command. Includes small refactors of the related REASSIGN OWNED BY command such as appending "_by" to some instances of "reassign_owned" for readability. Plan to follow up with implementation of DROP OWNED BY for feature parity with PostgresQL, which will resolve the issue linked. Release note: None --- docs/generated/sql/bnf/stmt_block.bnf | 18 ++++--- pkg/sql/drop_owned_by.go | 40 ++++++++++++++++ pkg/sql/opaque.go | 11 +++-- pkg/sql/parser/help_test.go | 1 + pkg/sql/parser/parse_test.go | 6 +++ pkg/sql/parser/sql.y | 50 +++++++++++++------- pkg/sql/reassign_owned_by.go | 2 +- pkg/sql/sem/tree/drop_owned_by.go | 34 +++++++++++++ pkg/sql/sem/tree/reassign_owned_by.go | 2 + pkg/sql/sem/tree/stmt.go | 11 ++++- pkg/sql/sqltelemetry/drop_owned_by.go | 18 +++++++ pkg/sql/testdata/telemetry/drop_owned_by | 19 ++++++++ pkg/sql/testdata/telemetry/reassign_owned_by | 1 - pkg/sql/walk.go | 1 + 14 files changed, 183 insertions(+), 31 deletions(-) create mode 100644 pkg/sql/drop_owned_by.go create mode 100644 pkg/sql/sem/tree/drop_owned_by.go create mode 100644 pkg/sql/sqltelemetry/drop_owned_by.go create mode 100644 pkg/sql/testdata/telemetry/drop_owned_by diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf index 4325dbc16e06..ba7e9097ff41 100644 --- a/docs/generated/sql/bnf/stmt_block.bnf +++ b/docs/generated/sql/bnf/stmt_block.bnf @@ -14,7 +14,8 @@ stmt ::= | prepare_stmt | revoke_stmt | savepoint_stmt - | reassign_owned_stmt + | reassign_owned_by_stmt + | drop_owned_by_stmt | release_stmt | refresh_stmt | nonpreparable_set_stmt @@ -90,9 +91,12 @@ revoke_stmt ::= savepoint_stmt ::= 'SAVEPOINT' name -reassign_owned_stmt ::= +reassign_owned_by_stmt ::= 'REASSIGN' 'OWNED' 'BY' role_spec_list 'TO' role_spec +drop_owned_by_stmt ::= + 'DROP' 'OWNED' 'BY' role_spec_list opt_drop_behavior + release_stmt ::= 'RELEASE' savepoint_name @@ -314,6 +318,11 @@ role_spec ::= | 'CURRENT_USER' | 'SESSION_USER' +opt_drop_behavior ::= + 'CASCADE' + | 'RESTRICT' + | + savepoint_name ::= 'SAVEPOINT' name | name @@ -709,11 +718,6 @@ opt_table ::= relation_expr_list ::= ( relation_expr ) ( ( ',' relation_expr ) )* -opt_drop_behavior ::= - 'CASCADE' - | 'RESTRICT' - | - set_clause_list ::= ( set_clause ) ( ( ',' set_clause ) )* diff --git a/pkg/sql/drop_owned_by.go b/pkg/sql/drop_owned_by.go new file mode 100644 index 000000000000..ba9bfafa0e0b --- /dev/null +++ b/pkg/sql/drop_owned_by.go @@ -0,0 +1,40 @@ +// Copyright 2020 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package sql + +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/server/telemetry" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" + "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" +) + +// dropOwnedByNode represents a DROP OWNED BY statement. +type dropOwnedByNode struct { + // TODO(angelaw): Uncomment when implementing - commenting out due to linting error. + // n *tree.DropOwnedBy +} + +func (p *planner) DropOwnedBy(ctx context.Context) (planNode, error) { + telemetry.Inc(sqltelemetry.CreateDropOwnedByCounter()) + // TODO(angelaw): Implementation. + return nil, unimplemented.NewWithIssue(55381, "drop owned by is not yet implemented") +} + +func (n *dropOwnedByNode) startExec(params runParams) error { + // TODO(angelaw): Implementation. + return nil +} +func (n *dropOwnedByNode) Next(runParams) (bool, error) { return false, nil } +func (n *dropOwnedByNode) Values() tree.Datums { return tree.Datums{} } +func (n *dropOwnedByNode) Close(context.Context) {} diff --git a/pkg/sql/opaque.go b/pkg/sql/opaque.go index e980d91a1ccb..c23b31e64532 100644 --- a/pkg/sql/opaque.go +++ b/pkg/sql/opaque.go @@ -105,18 +105,20 @@ func buildOpaque( plan, err = p.DropDatabase(ctx, n) case *tree.DropIndex: plan, err = p.DropIndex(ctx, n) + case *tree.DropOwnedBy: + plan, err = p.DropOwnedBy(ctx) case *tree.DropRole: plan, err = p.DropRole(ctx, n) case *tree.DropSchema: plan, err = p.DropSchema(ctx, n) + case *tree.DropSequence: + plan, err = p.DropSequence(ctx, n) case *tree.DropTable: plan, err = p.DropTable(ctx, n) case *tree.DropType: plan, err = p.DropType(ctx, n) case *tree.DropView: plan, err = p.DropView(ctx, n) - case *tree.DropSequence: - plan, err = p.DropSequence(ctx, n) case *tree.Grant: plan, err = p.Grant(ctx, n) case *tree.GrantRole: @@ -219,12 +221,13 @@ func init() { &tree.Discard{}, &tree.DropDatabase{}, &tree.DropIndex{}, + &tree.DropOwnedBy{}, + &tree.DropRole{}, &tree.DropSchema{}, + &tree.DropSequence{}, &tree.DropTable{}, &tree.DropType{}, &tree.DropView{}, - &tree.DropRole{}, - &tree.DropSequence{}, &tree.Grant{}, &tree.GrantRole{}, &tree.ReassignOwnedBy{}, diff --git a/pkg/sql/parser/help_test.go b/pkg/sql/parser/help_test.go index cec2369b7caa..46fca9406eed 100644 --- a/pkg/sql/parser/help_test.go +++ b/pkg/sql/parser/help_test.go @@ -246,6 +246,7 @@ func TestContextualHelp(t *testing.T) { {`REASSIGN OWNED BY ?? TO ??`, `REASSIGN OWNED BY`}, {`REASSIGN OWNED BY foo, bar TO ??`, `REASSIGN OWNED BY`}, + {`DROP OWNED BY ??`, `DROP OWNED BY`}, {`RESUME ??`, `RESUME`}, {`RESUME JOB ??`, `RESUME JOBS`}, diff --git a/pkg/sql/parser/parse_test.go b/pkg/sql/parser/parse_test.go index 1a002a239a83..61b29c7aa65d 100644 --- a/pkg/sql/parser/parse_test.go +++ b/pkg/sql/parser/parse_test.go @@ -1555,6 +1555,10 @@ func TestParse(t *testing.T) { {`REASSIGN OWNED BY foo TO bar`}, {`REASSIGN OWNED BY foo, bar TO third`}, + {`DROP OWNED BY foo`}, + {`DROP OWNED BY foo, bar`}, + {`DROP OWNED BY foo CASCADE`}, + {`DROP OWNED BY foo RESTRICT`}, {`COMMENT ON COLUMN a.b IS 'a'`}, {`COMMENT ON COLUMN a.b IS NULL`}, @@ -2673,6 +2677,8 @@ SKIP_MISSING_FOREIGN_KEYS, SKIP_MISSING_SEQUENCES, SKIP_MISSING_SEQUENCE_OWNERS, {`REASSIGN OWNED BY CURRENT_USER TO foo`, `REASSIGN OWNED BY "current_user" TO foo`}, {`REASSIGN OWNED BY SESSION_USER TO foo`, `REASSIGN OWNED BY "session_user" TO foo`}, + {`DROP OWNED BY CURRENT_USER`, `DROP OWNED BY "current_user"`}, + {`DROP OWNED BY SESSION_USER`, `DROP OWNED BY "session_user"`}, // Validate that GRANT and REVOKE can accept optional PRIVILEGES syntax {`GRANT ALL PRIVILEGES ON DATABASE foo TO root`, `GRANT ALL ON DATABASE foo TO root`}, diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index f40ca39e5217..024cadaea38f 100644 --- a/pkg/sql/parser/sql.y +++ b/pkg/sql/parser/sql.y @@ -828,7 +828,8 @@ func (u *sqlSymUnion) survive() tree.Survive { %type import_stmt %type pause_stmt pause_jobs_stmt pause_schedules_stmt %type <*tree.Select> for_schedules_clause -%type reassign_owned_stmt +%type reassign_owned_by_stmt +%type drop_owned_by_stmt %type release_stmt %type reset_stmt reset_session_stmt reset_csetting_stmt %type resume_stmt resume_jobs_stmt resume_schedules_stmt @@ -1254,22 +1255,23 @@ stmt_block: stmt: HELPTOKEN { return helpWith(sqllex, "") } -| preparable_stmt // help texts in sub-rule -| analyze_stmt // EXTEND WITH HELP: ANALYZE +| preparable_stmt // help texts in sub-rule +| analyze_stmt // EXTEND WITH HELP: ANALYZE | copy_from_stmt | comment_stmt -| execute_stmt // EXTEND WITH HELP: EXECUTE -| deallocate_stmt // EXTEND WITH HELP: DEALLOCATE -| discard_stmt // EXTEND WITH HELP: DISCARD -| grant_stmt // EXTEND WITH HELP: GRANT -| prepare_stmt // EXTEND WITH HELP: PREPARE -| revoke_stmt // EXTEND WITH HELP: REVOKE -| savepoint_stmt // EXTEND WITH HELP: SAVEPOINT -| reassign_owned_stmt // EXTEND WITH HELP: REASSIGN OWNED BY -| release_stmt // EXTEND WITH HELP: RELEASE -| refresh_stmt // EXTEND WITH HELP: REFRESH -| nonpreparable_set_stmt // help texts in sub-rule -| transaction_stmt // help texts in sub-rule +| execute_stmt // EXTEND WITH HELP: EXECUTE +| deallocate_stmt // EXTEND WITH HELP: DEALLOCATE +| discard_stmt // EXTEND WITH HELP: DISCARD +| grant_stmt // EXTEND WITH HELP: GRANT +| prepare_stmt // EXTEND WITH HELP: PREPARE +| revoke_stmt // EXTEND WITH HELP: REVOKE +| savepoint_stmt // EXTEND WITH HELP: SAVEPOINT +| reassign_owned_by_stmt // EXTEND WITH HELP: REASSIGN OWNED BY +| drop_owned_by_stmt // EXTEND WITH HELP: DROP OWNED BY +| release_stmt // EXTEND WITH HELP: RELEASE +| refresh_stmt // EXTEND WITH HELP: REFRESH +| nonpreparable_set_stmt // help texts in sub-rule +| transaction_stmt // help texts in sub-rule | close_cursor_stmt | declare_cursor_stmt | reindex_stmt @@ -7723,7 +7725,8 @@ multiple_set_clause: // %Category: Priv // %Text: REASSIGN OWNED BY { | CURRENT_USER | SESSION_USER}[,...] // TO { | CURRENT_USER | SESSION_USER} -reassign_owned_stmt: +// %SeeAlso: DROP OWNED BY +reassign_owned_by_stmt: REASSIGN OWNED BY role_spec_list TO role_spec { $$.val = &tree.ReassignOwnedBy{ @@ -7733,6 +7736,21 @@ reassign_owned_stmt: } | REASSIGN OWNED BY error // SHOW HELP: REASSIGN OWNED BY +// %Help: DROP OWNED BY - remove database objects owned by role(s). +// %Category: Priv +// %Text: DROP OWNED BY { | CURRENT_USER | SESSION_USER}[,...] +// [RESTRICT | CASCADE] +// %SeeAlso: REASSIGN OWNED BY +drop_owned_by_stmt: + DROP OWNED BY role_spec_list opt_drop_behavior +{ + $$.val = &tree.DropOwnedBy{ + Roles: $4.strs(), + DropBehavior: $5.dropBehavior(), + } +} +| DROP OWNED BY error // SHOW HELP: DROP OWNED BY + // A complete SELECT statement looks like this. // // The rule returns either a single select_stmt node or a tree of them, diff --git a/pkg/sql/reassign_owned_by.go b/pkg/sql/reassign_owned_by.go index e27758c1f0ac..13f0ed1fbbb7 100644 --- a/pkg/sql/reassign_owned_by.go +++ b/pkg/sql/reassign_owned_by.go @@ -25,7 +25,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" ) -// ReassignOwnedByNode represents a REASSIGN OWNED BY TO statement. +// ReassignOwnedByNode represents a REASSIGN OWNED BY TO statement. type reassignOwnedByNode struct { n *tree.ReassignOwnedBy } diff --git a/pkg/sql/sem/tree/drop_owned_by.go b/pkg/sql/sem/tree/drop_owned_by.go new file mode 100644 index 000000000000..2ca9766dc986 --- /dev/null +++ b/pkg/sql/sem/tree/drop_owned_by.go @@ -0,0 +1,34 @@ +// Copyright 2020 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package tree + +// DropOwnedBy represents a DROP OWNED BY command. +type DropOwnedBy struct { + Roles []string + DropBehavior DropBehavior +} + +var _ Statement = &DropOwnedBy{} + +// Format implements the NodeFormatter interface. +func (node *DropOwnedBy) Format(ctx *FmtCtx) { + ctx.WriteString("DROP OWNED BY ") + for i := range node.Roles { + if i > 0 { + ctx.WriteString(", ") + } + ctx.FormatNameP(&node.Roles[i]) + } + if node.DropBehavior != DropDefault { + ctx.WriteString(" ") + ctx.WriteString(node.DropBehavior.String()) + } +} diff --git a/pkg/sql/sem/tree/reassign_owned_by.go b/pkg/sql/sem/tree/reassign_owned_by.go index aaad69e791ff..ff1e218545b9 100644 --- a/pkg/sql/sem/tree/reassign_owned_by.go +++ b/pkg/sql/sem/tree/reassign_owned_by.go @@ -16,6 +16,8 @@ type ReassignOwnedBy struct { NewRole string } +var _ Statement = &ReassignOwnedBy{} + // Format implements the NodeFormatter interface. func (node *ReassignOwnedBy) Format(ctx *FmtCtx) { ctx.WriteString("REASSIGN OWNED BY ") diff --git a/pkg/sql/sem/tree/stmt.go b/pkg/sql/sem/tree/stmt.go index e8beb6a85eb5..aa3c0e34a59f 100644 --- a/pkg/sql/sem/tree/stmt.go +++ b/pkg/sql/sem/tree/stmt.go @@ -602,7 +602,13 @@ func (*Prepare) StatementTag() string { return "PREPARE" } func (*ReassignOwnedBy) StatementType() StatementType { return DDL } // StatementTag returns a short string identifying the type of statement. -func (*ReassignOwnedBy) StatementTag() string { return "REASSIGN OWNED" } +func (*ReassignOwnedBy) StatementTag() string { return "REASSIGN OWNED BY" } + +// StatementType implements the Statement interface. +func (*DropOwnedBy) StatementType() StatementType { return DDL } + +// StatementTag returns a short string identifying the type of statement. +func (*DropOwnedBy) StatementTag() string { return "DROP OWNED BY" } // StatementType implements the Statement interface. func (*RefreshMaterializedView) StatementType() StatementType { return DDL } @@ -1094,11 +1100,12 @@ func (n *Deallocate) String() string { return AsString(n) } func (n *Delete) String() string { return AsString(n) } func (n *DropDatabase) String() string { return AsString(n) } func (n *DropIndex) String() string { return AsString(n) } +func (n *DropOwnedBy) String() string { return AsString(n) } func (n *DropSchema) String() string { return AsString(n) } +func (n *DropSequence) String() string { return AsString(n) } func (n *DropTable) String() string { return AsString(n) } func (n *DropType) String() string { return AsString(n) } func (n *DropView) String() string { return AsString(n) } -func (n *DropSequence) String() string { return AsString(n) } func (n *DropRole) String() string { return AsString(n) } func (n *Execute) String() string { return AsString(n) } func (n *Explain) String() string { return AsString(n) } diff --git a/pkg/sql/sqltelemetry/drop_owned_by.go b/pkg/sql/sqltelemetry/drop_owned_by.go new file mode 100644 index 000000000000..a8aff9c8f5b4 --- /dev/null +++ b/pkg/sql/sqltelemetry/drop_owned_by.go @@ -0,0 +1,18 @@ +// Copyright 2020 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package sqltelemetry + +import "github.com/cockroachdb/cockroach/pkg/server/telemetry" + +// CreateDropOwnedByCounter returns a counter to increment for the DROP OWNED BY command. +func CreateDropOwnedByCounter() telemetry.Counter { + return telemetry.GetCounter("sql.drop_owned_by") +} diff --git a/pkg/sql/testdata/telemetry/drop_owned_by b/pkg/sql/testdata/telemetry/drop_owned_by new file mode 100644 index 000000000000..1da9d9586212 --- /dev/null +++ b/pkg/sql/testdata/telemetry/drop_owned_by @@ -0,0 +1,19 @@ +# This file contains telemetry tests for the sql.drop_owned_by counter. + +feature-allowlist +sql.drop_owned_by.* +---- + +exec +CREATE ROLE testuser; +CREATE TABLE t(); +GRANT CREATE ON DATABASE defaultdb TO testuser; +ALTER TABLE t OWNER TO testuser +---- + +# TODO(angelaw): Remove unimplemented message after implementation. +feature-usage +DROP OWNED BY testuser +---- +error: pq: unimplemented: drop owned by is not yet implemented +sql.drop_owned_by diff --git a/pkg/sql/testdata/telemetry/reassign_owned_by b/pkg/sql/testdata/telemetry/reassign_owned_by index ff6067d11c4f..d931182378f3 100644 --- a/pkg/sql/testdata/telemetry/reassign_owned_by +++ b/pkg/sql/testdata/telemetry/reassign_owned_by @@ -8,7 +8,6 @@ exec CREATE ROLE testuser; CREATE ROLE testuser2; CREATE TABLE t(); -GRANT testuser, testuser2 TO root; GRANT CREATE ON DATABASE defaultdb TO testuser, testuser2; ALTER TABLE t OWNER TO testuser ---- diff --git a/pkg/sql/walk.go b/pkg/sql/walk.go index 84943329f757..655db39bd6dd 100644 --- a/pkg/sql/walk.go +++ b/pkg/sql/walk.go @@ -395,6 +395,7 @@ var planNodeNames = map[reflect.Type]string{ reflect.TypeOf(&ordinalityNode{}): "ordinality", reflect.TypeOf(&projectSetNode{}): "project set", reflect.TypeOf(&reassignOwnedByNode{}): "reassign owned by", + reflect.TypeOf(&dropOwnedByNode{}): "drop owned by", reflect.TypeOf(&recursiveCTENode{}): "recursive cte", reflect.TypeOf(&refreshMaterializedViewNode{}): "refresh materialized view", reflect.TypeOf(&relocateNode{}): "relocate",