Merge pull request #81505 from cockroachdb/blathers/backport-release-…

…22.1-81420

release-22.1: parser: disallow 'SET tracing' in some contexts

docs/generated/sql/bnf/stmt_block.bnf

@@ -1262,6 +1262,7 @@ unreserved_keyword ::=
 	| 'TEXT'
 	| 'TIES'
 	| 'TRACE'
+	| 'TRACING'
 	| 'TRANSACTION'
 	| 'TRANSACTIONS'
 	| 'TRANSFER'

pkg/sql/alter_role.go

@@ -366,7 +366,9 @@ func (p *planner) processSetOrResetClause(
 	// default settings are stored per-database.
 	// The "role" setting can't be configured here, since we are already
 	// that role.
-	case "database", "role":
+	// The "tracing" setting is handled specially in the grammar, so we skip it
+	// here since it doesn't make sense to set as a default anyway.
+	case "database", "role", "tracing":
 		return unknown, "", sessionVar{}, nil, newCannotChangeParameterError(varName)
 	}
 	_, sVar, err = getSessionVar(varName, false /* missingOk */)

pkg/sql/lexbase/predicates.go

@@ -59,6 +59,8 @@ func init() {
 		"role",
 		"user",
 		"on",
+		"tenant",
+		"set",
 	} {
 		reservedOrLookaheadKeywords[s] = struct{}{}
 	}

pkg/sql/logictest/testdata/logic_test/alter_role_set

@@ -210,3 +210,7 @@ query OTT colnames
 SELECT database_id, role_name, settings FROM system.database_role_settings ORDER BY 1, 2
 ----
 database_id  role_name  settings
+
+# Regression test for the special "tracing" variable.
+query error parameter \"tracing\" cannot be changed
+ALTER ROLE ALL SET tracing = 'off'

pkg/sql/logictest/testdata/logic_test/set

@@ -748,6 +748,11 @@ query T
 SELECT name FROM pg_catalog.pg_settings WHERE name LIKE 'custom_option.%'
 ----
 
+query T
+SHOW tracing
+----
+off
+
 query T
 SHOW tracing.custom
 ----

pkg/sql/logictest/testdata/logic_test/set_local

@@ -546,3 +546,7 @@ query T
 SHOW custom_option.session_setting
 ----
 def
+
+# Regression test for the special "tracing" variable.
+query error parameter \"tracing\" cannot be changed
+SET LOCAL tracing = 'off'

pkg/sql/parser/lexer.go

@@ -80,76 +80,96 @@ 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, TENANT:
-		nextID := int32(0)
+	case NOT, WITH, AS, GENERATED, NULLS, RESET, ROLE, USER, ON, TENANT, SET:
+		nextToken := sqlSymType{}
 		if l.lastPos+1 < len(l.tokens) {
-			nextID = l.tokens[l.lastPos+1].id
+			nextToken = l.tokens[l.lastPos+1]
 		}
-		secondID := int32(0)
+		secondToken := sqlSymType{}
 		if l.lastPos+2 < len(l.tokens) {
-			secondID = l.tokens[l.lastPos+2].id
+			secondToken = l.tokens[l.lastPos+2]
+		}
+		thirdToken := sqlSymType{}
+		if l.lastPos+3 < len(l.tokens) {
+			thirdToken = l.tokens[l.lastPos+3]
 		}
 
 		// If you update these cases, update lex.lookaheadKeywords.
 		switch lval.id {
 		case AS:
-			switch nextID {
+			switch nextToken.id {
 			case OF:
 				lval.id = AS_LA
 			}
 		case NOT:
-			switch nextID {
+			switch nextToken.id {
 			case BETWEEN, IN, LIKE, ILIKE, SIMILAR:
 				lval.id = NOT_LA
 			}
 		case GENERATED:
-			switch nextID {
+			switch nextToken.id {
 			case ALWAYS:
 				lval.id = GENERATED_ALWAYS
 			case BY:
 				lval.id = GENERATED_BY_DEFAULT
 			}
 
 		case WITH:
-			switch nextID {
+			switch nextToken.id {
 			case TIME, ORDINALITY, BUCKET_COUNT:
 				lval.id = WITH_LA
 			}
 		case NULLS:
-			switch nextID {
+			switch nextToken.id {
 			case FIRST, LAST:
 				lval.id = NULLS_LA
 			}
 		case RESET:
-			switch nextID {
+			switch nextToken.id {
 			case ALL:
 				lval.id = RESET_ALL
 			}
 		case ROLE:
-			switch nextID {
+			switch nextToken.id {
 			case ALL:
 				lval.id = ROLE_ALL
 			}
 		case USER:
-			switch nextID {
+			switch nextToken.id {
 			case ALL:
 				lval.id = USER_ALL
 			}
 		case ON:
-			switch nextID {
+			switch nextToken.id {
 			case DELETE:
 				lval.id = ON_LA
 			case UPDATE:
-				switch secondID {
+				switch secondToken.id {
 				case NO, RESTRICT, CASCADE, SET:
 					lval.id = ON_LA
 				}
 			}
 		case TENANT:
-			switch nextID {
+			switch nextToken.id {
 			case ALL:
 				lval.id = TENANT_ALL
 			}
+		case SET:
+			switch nextToken.id {
+			case TRACING:
+				// Do not use the lookahead rule for `SET tracing.custom ...`
+				if secondToken.str != "." {
+					lval.id = SET_TRACING
+				}
+			case SESSION:
+				switch secondToken.id {
+				case TRACING:
+					// Do not use the lookahead rule for `SET SESSION tracing.custom ...`
+					if thirdToken.str != "." {
+						lval.id = SET_TRACING
+					}
+				}
+			}
 		}
 	}
 

pkg/sql/parser/sql.y

@@ -927,7 +927,7 @@ func (u *sqlSymUnion) cursorStmt() tree.CursorStmt {
 // - 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 TENANT_ALL
+%token USER_ALL ON_LA TENANT_ALL SET_TRACING
 
 %union {
   id    int32
@@ -5107,7 +5107,19 @@ set_exprs_internal:
 // %SeeAlso: SHOW SESSION, RESET, DISCARD, SHOW, SET CLUSTER SETTING, SET TRANSACTION, SET LOCAL
 // WEBDOCS/set-vars.html
 set_session_stmt:
-  SET SESSION set_rest_more
+  SET_TRACING TRACING to_or_eq var_list
+	{
+    /* SKIP DOC */
+    // We need to recognize the "set tracing" specially here using syntax lookahead.
+    $$.val = &tree.SetTracing{Values: $4.exprs()}
+	}
+| SET_TRACING SESSION TRACING to_or_eq var_list
+	{
+    /* SKIP DOC */
+    // We need to recognize the "set tracing" specially here using syntax lookahead.
+    $$.val = &tree.SetTracing{Values: $5.exprs()}
+	}
+| SET SESSION set_rest_more
   {
     $$.val = $3.stmt()
   }
@@ -5168,14 +5180,7 @@ set_transaction_stmt:
 generic_set:
   var_name to_or_eq var_list
   {
-    // We need to recognize the "set tracing" specially here; couldn't make "set
-    // tracing" a different grammar rule because of ambiguity.
-    varName := $1.strs()
-    if len(varName) == 1 && varName[0] == "tracing" {
-      $$.val = &tree.SetTracing{Values: $3.exprs()}
-    } else {
-      $$.val = &tree.SetVar{Name: strings.Join($1.strs(), "."), Values: $3.exprs()}
-    }
+    $$.val = &tree.SetVar{Name: strings.Join($1.strs(), "."), Values: $3.exprs()}
   }
 
 set_rest:
@@ -5667,7 +5672,7 @@ session_var:
   {
     $$ = $1 + "." + strings.Join($2.strs(), ".")
   }
-// Although ALL, SESSION_USER, DATABASE, LC_COLLATE, and LC_CTYPE are
+// Although ALL, SESSION_USER, DATABASE, LC_COLLATE, LC_CTYPE, and TRACING are
 // identifiers for the purpose of SHOW, they lex as separate token types, so
 // they need separate rules.
 | ALL
@@ -5679,6 +5684,12 @@ session_var:
 | SESSION_USER
 | LC_COLLATE
 | LC_CTYPE
+| TRACING { /* SKIP DOC */ }
+| TRACING session_var_parts
+  {
+    /* SKIP DOC */
+    $$ = $1 + "." + strings.Join($2.strs(), ".")
+  }
 // TIME ZONE is special: it is two tokens, but is really the identifier "TIME ZONE".
 | TIME ZONE { $$ = "timezone" }
 | TIME error // SHOW HELP: SHOW SESSION
@@ -8246,11 +8257,20 @@ opt_in_database:
     $$ = ""
   }
 
+// This rule is used when SET is used as a clause in another statement,
+// like ALTER ROLE ... SET.
 set_or_reset_clause:
   SET set_rest
   {
     $$.val = $2.setVar()
   }
+| SET_TRACING set_rest
+  {
+    /* SKIP DOC */
+    // We need to recognize the "set tracing" specially here since we do a
+    // syntax lookahead and use a different token.
+    $$.val = $2.setVar()
+  }
 | RESET_ALL ALL
   {
     $$.val = &tree.SetVar{ResetAll: true}
@@ -14355,6 +14375,7 @@ unreserved_keyword:
 | TEXT
 | TIES
 | TRACE
+| TRACING
 | TRANSACTION
 | TRANSACTIONS
 | TRANSFER

pkg/sql/parser/testdata/alter_user

@@ -321,3 +321,11 @@ ALTER ROLE ALL IN DATABASE d SET application_name = 'app' -- normalized!
 ALTER ROLE ALL IN DATABASE d SET application_name = ('app') -- fully parenthesized
 ALTER ROLE ALL IN DATABASE d SET application_name = '_' -- literals removed
 ALTER ROLE ALL IN DATABASE _ SET application_name = 'app' -- identifiers removed
+
+parse
+ALTER USER foo SET tracing = 'off'
+----
+ALTER USER foo SET tracing = 'off'
+ALTER USER foo SET tracing = ('off') -- fully parenthesized
+ALTER USER foo SET tracing = '_' -- literals removed
+ALTER USER _ SET tracing = 'off' -- identifiers removed

pkg/sql/parser/testdata/set

@@ -579,3 +579,11 @@ SHOW "a.b.c" -- normalized!
 SHOW "a.b.c" -- fully parenthesized
 SHOW "a.b.c" -- literals removed
 SHOW "a.b.c" -- identifiers removed
+
+parse
+SET LOCAL tracing = 'off'
+----
+SET LOCAL tracing = 'off'
+SET LOCAL tracing = ('off') -- fully parenthesized
+SET LOCAL tracing = '_' -- literals removed
+SET LOCAL tracing = 'off' -- identifiers removed