Merge #67955 #68001

67955: roachtest: remove passing test from ruby-pg blocklist r=RichardJCai a=rafiss fixes #67912 Release note: None 68001: sql/parser: add syntax for ALTER ROLE ... SET r=RichardJCai a=rafiss touches #21151 Note that the telemetry re-parsing logic does not work. This is consistent with the existing ALTER ROLE statement, and should be fixed separately. Release note (sql change): Add syntax for `ALTER ROLE ... SET` statements. The business logic for these statements is not implemented yet, but a later commit will add it. The following forms are supported: - ALTER ROLE { name | ALL } [ IN DATABASE database_name ] SET var { TO | = } { value | DEFAULT } - ALTER ROLE { name | ALL } [ IN DATABASE database_name ] RESET var - ALTER ROLE { name | ALL } [ IN DATABASE database_name ] RESET ALL As with other statements, the keywords ROLE and USER are interchangeable. This matches the PostgreSQL syntax: https://www.postgresql.org/docs/13/sql-alterrole.html Co-authored-by: Rafi Shamim <[email protected]>
cockroachdb · Jul 28, 2021 · e4cdcf4 · e4cdcf4
3 parents f45df6c + 87e2f98 + c59f340
commit e4cdcf4
Show file tree

Hide file tree

Showing 19 changed files with 454 additions and 76 deletions.
diff --git a/docs/generated/sql/bnf/alter_role_stmt.bnf b/docs/generated/sql/bnf/alter_role_stmt.bnf
@@ -7,3 +7,9 @@ alter_role_stmt ::=
 	| 'ALTER' 'ROLE' 'IF' 'EXISTS' name 
 	| 'ALTER' 'USER' 'IF' 'EXISTS' name opt_with role_options
 	| 'ALTER' 'USER' 'IF' 'EXISTS' name 
+	| 'ALTER' 'ROLE' name opt_in_database set_or_reset_clause
+	| 'ALTER' 'USER' name opt_in_database set_or_reset_clause
+	| 'ALTER' 'ROLE' 'IF' 'EXISTS' name opt_in_database set_or_reset_clause
+	| 'ALTER' 'USER' 'IF' 'EXISTS' name opt_in_database set_or_reset_clause
+	| 'ALTER' 'ROLE_ALL' 'ALL' opt_in_database set_or_reset_clause
+	| 'ALTER' 'USER_ALL' 'ALL' opt_in_database set_or_reset_clause
diff --git a/docs/generated/sql/bnf/set_rest.bnf b/docs/generated/sql/bnf/set_rest.bnf
@@ -0,0 +1,2 @@
+set_rest ::=
+	generic_set
diff --git a/docs/generated/sql/bnf/set_rest_more.bnf b/docs/generated/sql/bnf/set_rest_more.bnf
@@ -1,2 +1,2 @@
 set_rest_more ::=
-	generic_set
+	set_rest
diff --git a/docs/generated/sql/bnf/set_var.bnf b/docs/generated/sql/bnf/set_var.bnf
@@ -1,5 +1,5 @@
 preparable_set_stmt ::=
-	'SET' ( 'SESSION' | ) var_name '=' var_value ( ( ',' var_value ) )*
-	| 'SET' ( 'SESSION' | ) var_name 'TO' var_value ( ( ',' var_value ) )*
+	'SET' 'SESSION' set_rest
+	| 'SET' set_rest
 	| set_csetting_stmt
 	| use_stmt
diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf
@@ -384,6 +384,10 @@ alter_ddl_stmt ::=
 alter_role_stmt ::=
 	'ALTER' role_or_group_or_user string_or_placeholder opt_role_options
 	| 'ALTER' role_or_group_or_user 'IF' 'EXISTS' string_or_placeholder opt_role_options
+	| 'ALTER' role_or_group_or_user string_or_placeholder opt_in_database set_or_reset_clause
+	| 'ALTER' role_or_group_or_user 'IF' 'EXISTS' string_or_placeholder opt_in_database set_or_reset_clause
+	| 'ALTER' 'ROLE_ALL' 'ALL' opt_in_database set_or_reset_clause
+	| 'ALTER' 'USER_ALL' 'ALL' opt_in_database set_or_reset_clause
 
 opt_backup_targets ::=
 	targets
@@ -1324,6 +1328,15 @@ opt_role_options ::=
 	opt_with role_options
 	| 
 
+opt_in_database ::=
+	'IN' 'DATABASE' database_name
+	| 
+
+set_or_reset_clause ::=
+	'SET' set_rest
+	| 'RESET_ALL' 'ALL'
+	| 'RESET' session_var
+
 as_of_clause ::=
 	'AS' 'OF' 'SYSTEM' 'TIME' a_expr
 
@@ -1536,7 +1549,7 @@ opt_for_locking_clause ::=
 	| 
 
 set_rest_more ::=
-	generic_set
+	set_rest
 
 to_or_eq ::=
 	'='
@@ -1805,6 +1818,9 @@ abbreviated_revoke_stmt ::=
 role_options ::=
 	( role_option ) ( ( role_option ) )*
 
+set_rest ::=
+	generic_set
+
 backup_options ::=
 	'ENCRYPTION_PASSPHRASE' '=' string_or_placeholder
 	| 'REVISION_HISTORY'
@@ -2088,9 +2104,6 @@ offset_clause ::=
 	'OFFSET' a_expr
 	| 'OFFSET' select_fetch_first_value row_or_rows
 
-generic_set ::=
-	var_name to_or_eq var_list
-
 extra_var_value ::=
 	'ON'
 	| cockroachdb_extra_reserved_keyword
@@ -2304,6 +2317,9 @@ role_option ::=
 	| password_clause
 	| valid_until_clause
 
+generic_set ::=
+	var_name to_or_eq var_list
+
 d_expr ::=
 	'ICONST'
 	| 'FCONST'
@@ -2513,9 +2529,6 @@ all_or_distinct ::=
 for_locking_item ::=
 	for_locking_strength opt_locked_rels opt_nowait_or_skip
 
-var_list ::=
-	( var_value ) ( ( ',' var_value ) )*
-
 opt_ordinality ::=
 	'WITH' 'ORDINALITY'
 	| 
@@ -2655,6 +2668,9 @@ valid_until_clause ::=
 	'VALID' 'UNTIL' string_or_placeholder
 	| 'VALID' 'UNTIL' 'NULL'
 
+var_list ::=
+	( var_value ) ( ( ',' var_value ) )*
+
 typed_literal ::=
 	func_name_no_crdb_extra 'SCONST'
 	| const_typename 'SCONST'

diff --git a/pkg/cmd/roachtest/tests/ruby_pg_blocklist.go b/pkg/cmd/roachtest/tests/ruby_pg_blocklist.go
@@ -109,11 +109,10 @@ var rubyPGBlockList21_2 = blocklist{
 	"PG::Connection type casting with default result type map can type cast #copy_data output with explicit decoder":                                                                                                            "unknown",
 	"PG::Connection type casting with default result type map should respect a type mapping for result":                                                                                                                         "unknown",
 	"PG::Connection type casting with default result type map should work with arbitrary number of params in conjunction with type casting":                                                                                     "unknown",
+	"PG::Result encapsulates PG_DIAG_SEVERITY_NONLOCALIZED error in a PG::Error object":                                                                                                                                         "unknown",
 	"PG::Result encapsulates database object names for integrity constraint violations":                                                                                                                                         "unknown",
 	"PG::Result encapsulates errors in a PG::Error object":                                                                                                                                                                      "unknown",
-	"PG::Result encapsulates PG_DIAG_SEVERITY_NONLOCALIZED error in a PG::Error object":                                                                                                                                         "unknown",
 	"PG::Result raises a proper exception for a nonexistant schema":                                                                                                                                                             "unknown",
-	"PG::TypeMapByClass should expire the cache after changes to the coders":                                                                                                                                                    "unknown",
 	"PG::TypeMapByColumn forwards get_copy_data conversions to another TypeMapByColumn as #default_type_map":                                                                                                                    "unknown",
 	"PG::TypeMapByColumn should gracefully handle not initialized state":                                                                                                                                                        "unknown",
 	"PG::TypeMapByColumn will deny copy queries with different column count":                                                                                                                                                    "unknown",

diff --git a/pkg/sql/lexbase/predicates.go b/pkg/sql/lexbase/predicates.go
@@ -55,6 +55,9 @@ func init() {
 		"similar",
 		"time",
 		"generated",
+		"reset",
+		"role",
+		"user",
 	} {
 		reservedOrLookaheadKeywords[s] = struct{}{}
 	}

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/explain b/pkg/sql/opt/exec/execbuilder/testdata/explain
@@ -521,7 +521,7 @@ vectorized: true
         │ group by: username
         │
         └── • sort
-            │ order: +role
+            │ order: +"role"
             │
             └── • hash join (left outer)
                 │ equality: (username) = (member)

diff --git a/pkg/sql/parser/lexer.go b/pkg/sql/parser/lexer.go
@@ -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:
+	case NOT, WITH, AS, GENERATED, NULLS, RESET, ROLE, USER:
 		nextID := int32(0)
 		if l.lastPos+1 < len(l.tokens) {
 			nextID = l.tokens[l.lastPos+1].id
@@ -114,6 +114,21 @@ func (l *lexer) Lex(lval *sqlSymType) int {
 			case FIRST, LAST:
 				lval.id = NULLS_LA
 			}
+		case RESET:
+			switch nextID {
+			case ALL:
+				lval.id = RESET_ALL
+			}
+		case ROLE:
+			switch nextID {
+			case ALL:
+				lval.id = ROLE_ALL
+			}
+		case USER:
+			switch nextID {
+			case ALL:
+				lval.id = USER_ALL
+			}
 		}
 	}
 

diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y
@@ -720,6 +720,9 @@ func (u *sqlSymUnion) abbreviatedRevoke() tree.AbbreviatedRevoke {
 func (u *sqlSymUnion) alterDefaultPrivilegesTargetObject() tree.AlterDefaultPrivilegesTargetObject {
   return u.val.(tree.AlterDefaultPrivilegesTargetObject)
 }
+func (u *sqlSymUnion) setVar() *tree.SetVar {
+    return u.val.(*tree.SetVar)
+}
 %}
 
 // NB: the %token definitions must come before the %type definitions in this
@@ -852,8 +855,10 @@ func (u *sqlSymUnion) alterDefaultPrivilegesTargetObject() tree.AlterDefaultPriv
 // 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).
-%token NOT_LA NULLS_LA WITH_LA AS_LA GENERATED_ALWAYS
+// 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.
+%token NOT_LA NULLS_LA WITH_LA AS_LA GENERATED_ALWAYS RESET_ALL ROLE_ALL USER_ALL
 
 %union {
   id    int32
@@ -876,6 +881,7 @@ func (u *sqlSymUnion) alterDefaultPrivilegesTargetObject() tree.AlterDefaultPriv
 %type <tree.Statement> alter_range_stmt
 %type <tree.Statement> alter_partition_stmt
 %type <tree.Statement> alter_role_stmt
+%type <*tree.SetVar> set_or_reset_clause
 %type <tree.Statement> alter_type_stmt
 %type <tree.Statement> alter_schema_stmt
 %type <tree.Statement> alter_unsupported_stmt
@@ -1024,6 +1030,7 @@ func (u *sqlSymUnion) alterDefaultPrivilegesTargetObject() tree.AlterDefaultPriv
 %type <tree.Statement> set_exprs_internal
 %type <tree.Statement> generic_set
 %type <tree.Statement> set_rest_more
+%type <tree.Statement> set_rest
 %type <tree.Statement> set_names
 
 %type <tree.Statement> show_stmt
@@ -1134,7 +1141,7 @@ func (u *sqlSymUnion) alterDefaultPrivilegesTargetObject() tree.AlterDefaultPriv
 %type <*tree.UnresolvedName> func_name func_name_no_crdb_extra
 %type <str> opt_class opt_collate
 
-%type <str> cursor_name database_name index_name opt_index_name column_name insert_column_item statistics_name window_name
+%type <str> cursor_name database_name index_name opt_index_name column_name insert_column_item statistics_name window_name opt_in_database
 %type <str> family_name opt_family_name table_alias_name constraint_name target_name zone_name partition_name collation_name
 %type <str> db_object_name_component
 %type <*tree.UnresolvedObjectName> table_name db_name standalone_index_name sequence_name type_name view_name db_object_name simple_db_object_name complex_db_object_name
@@ -4574,7 +4581,7 @@ generic_set:
     }
   }
 
-set_rest_more:
+set_rest:
 // Generic SET syntaxes:
    generic_set
 // Special SET syntax forms in addition to the generic form.
@@ -4586,13 +4593,18 @@ set_rest_more:
     /* SKIP DOC */
     $$.val = &tree.SetVar{Name: "timezone", Values: tree.Exprs{$3.expr()}}
   }
+| var_name FROM CURRENT { return unimplemented(sqllex, "set from current") }
 // "SET SCHEMA 'value' is an alias for SET search_path TO value. Only
 // one schema can be specified using this syntax."
 | SCHEMA var_value
   {
     /* SKIP DOC */
     $$.val = &tree.SetVar{Name: "search_path", Values: tree.Exprs{$2.expr()}}
   }
+
+set_rest_more:
+// SET syntaxes supported as a clause of other statements:
+  set_rest
 | SESSION AUTHORIZATION DEFAULT
   {
     /* SKIP DOC */
@@ -4604,7 +4616,6 @@ set_rest_more:
   }
 // See comment for the non-terminal for SET NAMES below.
 | set_names
-| var_name FROM CURRENT { return unimplemented(sqllex, "set from current") }
 | error // SHOW HELP: SET SESSION
 
 // SET NAMES is the SQL standard syntax for SET client_encoding.
@@ -7179,7 +7190,10 @@ create_role_stmt:
 
 // %Help: ALTER ROLE - alter a role
 // %Category: Priv
-// %Text: ALTER ROLE <name> [WITH] <options...>
+// %Text:
+// ALTER ROLE <name> [WITH] <options...>
+// ALTER ROLE { name | ALL } [ IN DATABASE database_name ] SET var { TO | = } { value | DEFAULT }
+// ALTER ROLE { name | ALL } [ IN DATABASE database_name ] RESET { var | ALL }
 // %SeeAlso: CREATE ROLE, DROP ROLE, SHOW ROLES
 alter_role_stmt:
   ALTER role_or_group_or_user string_or_placeholder opt_role_options
@@ -7190,8 +7204,48 @@ alter_role_stmt:
 {
   $$.val = &tree.AlterRole{Name: $5.expr(), IfExists: true, KVOptions: $6.kvOptions(), IsRole: $2.bool()}
 }
+| ALTER role_or_group_or_user string_or_placeholder opt_in_database set_or_reset_clause
+  {
+    $$.val = &tree.AlterRoleSet{RoleName: $3.expr(), DatabaseName: tree.Name($4), IsRole: $2.bool(), SetOrReset: $5.setVar()}
+  }
+| ALTER role_or_group_or_user IF EXISTS string_or_placeholder opt_in_database set_or_reset_clause
+  {
+    $$.val = &tree.AlterRoleSet{RoleName: $5.expr(), IfExists: true, DatabaseName: tree.Name($6), IsRole: $2.bool(), SetOrReset: $7.setVar()}
+  }
+| ALTER ROLE_ALL ALL opt_in_database set_or_reset_clause
+  {
+    $$.val = &tree.AlterRoleSet{AllRoles: true, DatabaseName: tree.Name($4), IsRole: true, SetOrReset: $5.setVar()}
+  }
+| ALTER USER_ALL ALL opt_in_database set_or_reset_clause
+  {
+    $$.val = &tree.AlterRoleSet{AllRoles: true, DatabaseName: tree.Name($4), IsRole: false, SetOrReset: $5.setVar()}
+  }
 | ALTER role_or_group_or_user error // SHOW HELP: ALTER ROLE
 
+opt_in_database:
+  IN DATABASE database_name
+  {
+    $$ = $3
+  }
+| /* EMPTY */
+  {
+    $$ = ""
+  }
+
+set_or_reset_clause:
+  SET set_rest
+  {
+    $$.val = $2.setVar()
+  }
+| RESET_ALL ALL
+  {
+    $$.val = &tree.SetVar{ResetAll: true}
+  }
+| RESET session_var
+  {
+    $$.val = &tree.SetVar{Name: $2, Values:tree.Exprs{tree.DefaultVal{}}}
+  }
+
 // "CREATE GROUP is now an alias for CREATE ROLE"
 // https://www.postgresql.org/docs/10/static/sql-creategroup.html
 role_or_group_or_user: