Skip to content

Commit

Permalink
parser: adjust ALTER TENANT ALL syntax
Browse files Browse the repository at this point in the history
This change brings the syntax inline to what was proposed in the RFC.
This is worthwhile since it matches with the syntax we use in other
"ALL" cases -- notable `ALTER ROLE ALL SET ...``.

Release justification: low risk update to new functionality
Release note: None
  • Loading branch information
rafiss committed Mar 9, 2022
1 parent d384f95 commit 048854d
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 35 deletions.
2 changes: 1 addition & 1 deletion docs/generated/sql/bnf/alter_tenant_csetting_stmt.bnf
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
alter_tenant_csetting_stmt ::=
'ALTER' 'TENANT' d_expr set_or_reset_csetting_stmt
| 'ALTER' 'ALL' 'TENANTS' set_or_reset_csetting_stmt
| 'ALTER' 'TENANT_ALL' 'ALL' set_or_reset_csetting_stmt
3 changes: 1 addition & 2 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ alter_role_stmt ::=

alter_tenant_csetting_stmt ::=
'ALTER' 'TENANT' d_expr set_or_reset_csetting_stmt
| 'ALTER' 'ALL' 'TENANTS' set_or_reset_csetting_stmt
| 'ALTER' 'TENANT_ALL' 'ALL' set_or_reset_csetting_stmt

opt_backup_targets ::=
targets
Expand Down Expand Up @@ -1245,7 +1245,6 @@ unreserved_keyword ::=
| 'TEMPLATE'
| 'TEMPORARY'
| 'TENANT'
| 'TENANTS'
| 'TESTING_RELOCATE'
| 'TEXT'
| 'TIES'
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/cluster_settings
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,15 @@ ALTER TENANT 10 SET CLUSTER SETTING server.mem_profile.total_dump_size_limit='10

skipif config 3node-tenant
statement error unimplemented
ALTER ALL TENANTS SET CLUSTER SETTING server.mem_profile.total_dump_size_limit='10M'
ALTER TENANT ALL SET CLUSTER SETTING server.mem_profile.total_dump_size_limit='10M'

skipif config 3node-tenant
statement error unimplemented
ALTER TENANT 10 RESET CLUSTER SETTING server.mem_profile.total_dump_size_limit

skipif config 3node-tenant
statement error unimplemented
ALTER ALL TENANTS RESET CLUSTER SETTING server.mem_profile.total_dump_size_limit
ALTER TENANT ALL RESET CLUSTER SETTING server.mem_profile.total_dump_size_limit

statement error unimplemented
SHOW CLUSTER SETTING server.mem_profile.total_dump_size_limit FOR TENANT 10
Expand Down
8 changes: 5 additions & 3 deletions pkg/sql/parser/help_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,11 @@ func TestContextualHelp(t *testing.T) {
{`ALTER TABLE blah SPLIT AT (SELECT 1) ??`, `ALTER TABLE`},

{`ALTER TENANT 1 ??`, `ALTER TENANT`},
{`ALTER TENANT 1 ??`, `ALTER TENANT`},
{`ALTER ALL TENANTS ??`, `ALTER TENANT`},
{`ALTER ALL TENANTS ??`, `ALTER TENANT`},
{`ALTER TENANT 1 SET ??`, `ALTER TENANT`},
{`ALTER TENANT 1 RESET ??`, `ALTER TENANT`},
{`ALTER TENANT ALL ??`, `ALTER TENANT`},
{`ALTER TENANT ALL SET ??`, `ALTER TENANT`},
{`ALTER TENANT ALL RESET ??`, `ALTER TENANT`},

{`ALTER TYPE ??`, `ALTER TYPE`},
{`ALTER TYPE t ??`, `ALTER TYPE`},
Expand Down
7 changes: 6 additions & 1 deletion pkg/sql/parser/lexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (l *lexer) Lex(lval *sqlSymType) int {
*lval = l.tokens[l.lastPos]

switch lval.id {
case NOT, WITH, AS, GENERATED, NULLS, RESET, ROLE, USER, ON:
case NOT, WITH, AS, GENERATED, NULLS, RESET, ROLE, USER, ON, TENANT:
nextID := int32(0)
if l.lastPos+1 < len(l.tokens) {
nextID = l.tokens[l.lastPos+1].id
Expand Down Expand Up @@ -145,6 +145,11 @@ func (l *lexer) Lex(lval *sqlSymType) int {
lval.id = ON_LA
}
}
case TENANT:
switch nextID {
case ALL:
lval.id = TENANT_ALL
}
}
}

Expand Down
31 changes: 18 additions & 13 deletions pkg/sql/parser/sql.y
Original file line number Diff line number Diff line change
Expand Up @@ -903,16 +903,22 @@ func (u *sqlSymUnion) fetchCursor() *tree.FetchCursor {
// tokens when required (based on looking one token ahead).
// Reference: pkg/sql/parser/lexer.go
//
// NOT_LA exists so that productions such as NOT LIKE can be given the same
// - NOT_LA exists so that productions such as NOT LIKE can be given the same
// precedence as LIKE; otherwise they'd effectively have the same precedence as
// NOT, at least with respect to their left-hand subexpression. WITH_LA is
// needed to make the grammar LALR(1). GENERATED_ALWAYS is needed to support
// the Postgres syntax for computed columns along with our family related
// extensions (CREATE FAMILY/CREATE FAMILY family_name). RESET_ALL is used
// to differentiate `RESET var` from `RESET ALL`. ROLE_ALL and USER_ALL are
// used in ALTER ROLE statements that affect all roles.
// NOT, at least with respect to their left-hand subexpression.
// - WITH_LA is needed to make the grammar LALR(1).
// - GENERATED_ALWAYS is needed to support the Postgres syntax for computed
// columns along with our family related extensions (CREATE FAMILY/CREATE FAMILY
// family_name).
// - RESET_ALL is used to differentiate `RESET var` from `RESET ALL`.
// - ROLE_ALL and USER_ALL are used in ALTER ROLE statements that affect all
// roles.
// - ON_LA is needed for ON UPDATE and ON DELETE expressions for foreign key
// references.
// - TENANT_ALL is used to differentiate `ALTER TENANT <id>` from
// `ALTER TENANT ALL`.
%token NOT_LA NULLS_LA WITH_LA AS_LA GENERATED_ALWAYS GENERATED_BY_DEFAULT RESET_ALL ROLE_ALL
%token USER_ALL ON_LA
%token USER_ALL ON_LA TENANT_ALL

%union {
id int32
Expand Down Expand Up @@ -4954,8 +4960,8 @@ set_csetting_stmt:
// %Help: ALTER TENANT - alter tenant configuration
// %Category: Cfg
// %Text:
// ALTER { TENANT <tenant_id> | ALL TENANTS } SET CLUSTER SETTING <var> { TO | = } <value>
// ALTER { TENANT <tenant_id> | ALL TENANTS } RESET CLUSTER SETTING <var>
// ALTER TENANT { <tenant_id> | ALL } SET CLUSTER SETTING <var> { TO | = } <value>
// ALTER TENANT { <tenant_id> | ALL } RESET CLUSTER SETTING <var>
// %SeeAlso: SET CLUSTER SETTING
alter_tenant_csetting_stmt:
ALTER TENANT d_expr set_or_reset_csetting_stmt
Expand All @@ -4966,7 +4972,7 @@ alter_tenant_csetting_stmt:
TenantID: $3.expr(),
}
}
| ALTER ALL TENANTS set_or_reset_csetting_stmt
| ALTER TENANT_ALL ALL set_or_reset_csetting_stmt
{
csettingStmt := $4.stmt().(*tree.SetClusterSetting)
$$.val = &tree.AlterTenantSetClusterSetting{
Expand All @@ -4975,7 +4981,7 @@ alter_tenant_csetting_stmt:
}
}
| ALTER TENANT error // SHOW HELP: ALTER TENANT
| ALTER ALL TENANTS error // SHOW HELP: ALTER TENANT
| ALTER TENANT_ALL ALL error // SHOW HELP: ALTER TENANT
set_or_reset_csetting_stmt:
reset_csetting_stmt
Expand Down Expand Up @@ -14183,7 +14189,6 @@ unreserved_keyword:
| TEMPLATE
| TEMPORARY
| TENANT
| TENANTS
| TESTING_RELOCATE
| TEXT
| TIES
Expand Down
20 changes: 10 additions & 10 deletions pkg/sql/parser/testdata/alter_tenant
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ ALTER TENANT $1 SET CLUSTER SETTING a = _ -- literals removed
ALTER TENANT $1 SET CLUSTER SETTING a = 3 -- identifiers removed

parse
ALTER ALL TENANTS SET CLUSTER SETTING a = 3
ALTER TENANT ALL SET CLUSTER SETTING a = 3
----
ALTER ALL TENANTS SET CLUSTER SETTING a = 3
ALTER ALL TENANTS SET CLUSTER SETTING a = (3) -- fully parenthesized
ALTER ALL TENANTS SET CLUSTER SETTING a = _ -- literals removed
ALTER ALL TENANTS SET CLUSTER SETTING a = 3 -- identifiers removed
ALTER TENANT ALL SET CLUSTER SETTING a = 3
ALTER TENANT ALL SET CLUSTER SETTING a = (3) -- fully parenthesized
ALTER TENANT ALL SET CLUSTER SETTING a = _ -- literals removed
ALTER TENANT ALL SET CLUSTER SETTING a = 3 -- identifiers removed

parse
ALTER TENANT 123 RESET CLUSTER SETTING a
Expand All @@ -55,9 +55,9 @@ ALTER TENANT $1 SET CLUSTER SETTING a = DEFAULT -- literals removed
ALTER TENANT $1 SET CLUSTER SETTING a = DEFAULT -- identifiers removed

parse
ALTER ALL TENANTS RESET CLUSTER SETTING a
ALTER TENANT ALL RESET CLUSTER SETTING a
----
ALTER ALL TENANTS SET CLUSTER SETTING a = DEFAULT -- normalized!
ALTER ALL TENANTS SET CLUSTER SETTING a = (DEFAULT) -- fully parenthesized
ALTER ALL TENANTS SET CLUSTER SETTING a = DEFAULT -- literals removed
ALTER ALL TENANTS SET CLUSTER SETTING a = DEFAULT -- identifiers removed
ALTER TENANT ALL SET CLUSTER SETTING a = DEFAULT -- normalized!
ALTER TENANT ALL SET CLUSTER SETTING a = (DEFAULT) -- fully parenthesized
ALTER TENANT ALL SET CLUSTER SETTING a = DEFAULT -- literals removed
ALTER TENANT ALL SET CLUSTER SETTING a = DEFAULT -- identifiers removed
5 changes: 2 additions & 3 deletions pkg/sql/sem/tree/tenant_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,10 @@ type AlterTenantSetClusterSetting struct {

// Format implements the NodeFormatter interface.
func (n *AlterTenantSetClusterSetting) Format(ctx *FmtCtx) {
ctx.WriteString("ALTER ")
ctx.WriteString("ALTER TENANT ")
if n.TenantAll {
ctx.WriteString("ALL TENANTS")
ctx.WriteString("ALL")
} else {
ctx.WriteString("TENANT ")
ctx.FormatNode(n.TenantID)
}
ctx.WriteByte(' ')
Expand Down

0 comments on commit 048854d

Please sign in to comment.