sql/parser: reveal broken statement telemetry

The SQL layer generates  statement telemetry in two phases:

- first the literal constants are removed statements to form app stat
  keys
- then the app stat keys are re-parsed into an AST
- the new AST is then re-pretty-printed using anonymization

The problem here is that there is no guarantee that the result of
stripping literals forms a valid SQL syntax. Moreover, it's possible
that the result of stripping the literals generates syntax that's
different from the original SQL semantically.

This patch exposes this difference. All the diffs in this commit
should be interpreted as bugs that cause invalid data in the telemetry.

Release note: None

pkg/sql/parser/parse_test.go

@@ -56,11 +56,31 @@ func TestParseDatadriven(t *testing.T) {
 				var buf bytes.Buffer
 				fmt.Fprintf(&buf, "%s%s\n", ref, note)
 				fmt.Fprintln(&buf, stmts.StringWithFlags(tree.FmtAlwaysGroupExprs), "-- fully parenthetized")
-				fmt.Fprintln(&buf, stmts.StringWithFlags(tree.FmtHideConstants), "-- literals removed")
+				constantsHidden := stmts.StringWithFlags(tree.FmtHideConstants)
+				fmt.Fprintln(&buf, constantsHidden, "-- literals removed")
+
+				// As of this writing, the SQL statement stats proceed as follows:
+				// first the literals are removed from statement to form a stat key,
+				// then the stat key is re-parsed, to undergo the anonymization stage.
+				// We also want to check the re-parsing is fine.
+				//
+				// TODO(knz,rafiss): Turn the following two cases into proper test
+				// errors once the bugs are fixed.
+				reparsedStmts, err := parser.Parse(constantsHidden)
+				if err != nil {
+					fmt.Fprintln(&buf, "REPARSE WITHOUT LITERALS FAILS:", err)
+				} else {
+					reparsedStmtsS := reparsedStmts.String()
+					if reparsedStmtsS != constantsHidden {
+						fmt.Fprintln(&buf, reparsedStmtsS, "-- UNEXPECTED REPARSED AST WITHOUT LITERALS")
+					}
+				}
+
 				fmt.Fprintln(&buf, stmts.StringWithFlags(tree.FmtAnonymize), "-- identifiers removed")
 				if strings.Contains(ref, tree.PasswordSubstitution) {
 					fmt.Fprintln(&buf, stmts.StringWithFlags(tree.FmtShowPasswords), "-- passwords exposed")
 				}
+
 				return buf.String()
 			}
 			d.Fatalf(t, "unsupported command: %s", d.Cmd)

pkg/sql/parser/testdata/parse/alter_sequence

@@ -45,6 +45,7 @@ ALTER SEQUENCE a INCREMENT BY 5 START WITH 1000
 ALTER SEQUENCE a INCREMENT BY 5 START WITH 1000
 ALTER SEQUENCE a INCREMENT BY 5 START WITH 1000 -- fully parenthetized
 ALTER SEQUENCE a INCREMENT BY _ START WITH _ -- literals removed
+REPARSE WITHOUT LITERALS FAILS: at or near "_": syntax error
 ALTER SEQUENCE _ INCREMENT BY 5 START WITH 1000 -- identifiers removed
 
 parse
@@ -53,6 +54,7 @@ EXPLAIN ALTER SEQUENCE a INCREMENT BY 5 START WITH 1000
 EXPLAIN ALTER SEQUENCE a INCREMENT BY 5 START WITH 1000
 EXPLAIN ALTER SEQUENCE a INCREMENT BY 5 START WITH 1000 -- fully parenthetized
 EXPLAIN ALTER SEQUENCE a INCREMENT BY _ START WITH _ -- literals removed
+REPARSE WITHOUT LITERALS FAILS: at or near "_": syntax error
 EXPLAIN ALTER SEQUENCE _ INCREMENT BY 5 START WITH 1000 -- identifiers removed
 
 parse
@@ -61,6 +63,7 @@ ALTER SEQUENCE IF EXISTS a INCREMENT BY 5 START WITH 1000
 ALTER SEQUENCE IF EXISTS a INCREMENT BY 5 START WITH 1000
 ALTER SEQUENCE IF EXISTS a INCREMENT BY 5 START WITH 1000 -- fully parenthetized
 ALTER SEQUENCE IF EXISTS a INCREMENT BY _ START WITH _ -- literals removed
+REPARSE WITHOUT LITERALS FAILS: at or near "_": syntax error
 ALTER SEQUENCE IF EXISTS _ INCREMENT BY 5 START WITH 1000 -- identifiers removed
 
 parse
@@ -69,6 +72,7 @@ ALTER SEQUENCE IF EXISTS a NO CYCLE CACHE 1
 ALTER SEQUENCE IF EXISTS a NO CYCLE CACHE 1
 ALTER SEQUENCE IF EXISTS a NO CYCLE CACHE 1 -- fully parenthetized
 ALTER SEQUENCE IF EXISTS a NO CYCLE CACHE _ -- literals removed
+REPARSE WITHOUT LITERALS FAILS: at or near "_": syntax error
 ALTER SEQUENCE IF EXISTS _ NO CYCLE CACHE 1 -- identifiers removed
 
 parse

pkg/sql/parser/testdata/parse/alter_table

@@ -614,6 +614,7 @@ ALTER TABLE a SPLIT AT VALUES (1) WITH EXPIRATION TIMESTAMP '2200-01-01 00:00:00
 ALTER TABLE a SPLIT AT VALUES (1) WITH EXPIRATION TIMESTAMP '2200-01-01 00:00:00.0'
 ALTER TABLE a SPLIT AT VALUES ((1)) WITH EXPIRATION (TIMESTAMP ('2200-01-01 00:00:00.0')) -- fully parenthetized
 ALTER TABLE a SPLIT AT VALUES (_) WITH EXPIRATION TIMESTAMP _ -- literals removed
+REPARSE WITHOUT LITERALS FAILS: at or near "_": syntax error
 ALTER TABLE _ SPLIT AT VALUES (1) WITH EXPIRATION TIMESTAMP '2200-01-01 00:00:00.0' -- identifiers removed
 
 parse
@@ -630,6 +631,7 @@ ALTER TABLE a SPLIT AT VALUES (1) WITH EXPIRATION TIMESTAMPTZ '2200-01-01 00:00:
 ALTER TABLE a SPLIT AT VALUES (1) WITH EXPIRATION TIMESTAMPTZ '2200-01-01 00:00:00.0'
 ALTER TABLE a SPLIT AT VALUES ((1)) WITH EXPIRATION (TIMESTAMPTZ ('2200-01-01 00:00:00.0')) -- fully parenthetized
 ALTER TABLE a SPLIT AT VALUES (_) WITH EXPIRATION TIMESTAMPTZ _ -- literals removed
+REPARSE WITHOUT LITERALS FAILS: at or near "_": syntax error
 ALTER TABLE _ SPLIT AT VALUES (1) WITH EXPIRATION TIMESTAMPTZ '2200-01-01 00:00:00.0' -- identifiers removed
 
 parse

pkg/sql/parser/testdata/parse/alter_user

@@ -4,6 +4,7 @@ ALTER USER foo PASSWORD bar
 ALTER USER 'foo' WITH PASSWORD '*****' -- normalized!
 ALTER USER ('foo') WITH PASSWORD '*****' -- fully parenthetized
 ALTER USER _ WITH PASSWORD '*****' -- literals removed
+ALTER USER '_' WITH PASSWORD '*****' -- UNEXPECTED REPARSED AST WITHOUT LITERALS
 ALTER USER 'foo' WITH PASSWORD '*****' -- identifiers removed
 ALTER USER 'foo' WITH PASSWORD 'bar' -- passwords exposed
 
@@ -13,6 +14,7 @@ ALTER USER foo WITH PASSWORD bar
 ALTER USER 'foo' WITH PASSWORD '*****' -- normalized!
 ALTER USER ('foo') WITH PASSWORD '*****' -- fully parenthetized
 ALTER USER _ WITH PASSWORD '*****' -- literals removed
+ALTER USER '_' WITH PASSWORD '*****' -- UNEXPECTED REPARSED AST WITHOUT LITERALS
 ALTER USER 'foo' WITH PASSWORD '*****' -- identifiers removed
 ALTER USER 'foo' WITH PASSWORD 'bar' -- passwords exposed
 
@@ -22,6 +24,7 @@ ALTER USER foo WITH PASSWORD NULL
 ALTER USER 'foo' WITH PASSWORD NULL -- normalized!
 ALTER USER ('foo') WITH PASSWORD (NULL) -- fully parenthetized
 ALTER USER _ WITH PASSWORD _ -- literals removed
+ALTER USER '_' WITH PASSWORD '*****' -- UNEXPECTED REPARSED AST WITHOUT LITERALS
 ALTER USER 'foo' WITH PASSWORD NULL -- identifiers removed
 
 parse
@@ -30,6 +33,7 @@ ALTER ROLE foo WITH CREATEDB
 ALTER ROLE 'foo' WITH CREATEDB -- normalized!
 ALTER ROLE ('foo') WITH CREATEDB -- fully parenthetized
 ALTER ROLE _ WITH CREATEDB -- literals removed
+ALTER ROLE '_' WITH CREATEDB -- UNEXPECTED REPARSED AST WITHOUT LITERALS
 ALTER ROLE 'foo' WITH CREATEDB -- identifiers removed
 
 parse
@@ -38,6 +42,7 @@ ALTER ROLE foo CREATEDB
 ALTER ROLE 'foo' WITH CREATEDB -- normalized!
 ALTER ROLE ('foo') WITH CREATEDB -- fully parenthetized
 ALTER ROLE _ WITH CREATEDB -- literals removed
+ALTER ROLE '_' WITH CREATEDB -- UNEXPECTED REPARSED AST WITHOUT LITERALS
 ALTER ROLE 'foo' WITH CREATEDB -- identifiers removed
 
 parse
@@ -46,6 +51,7 @@ ALTER ROLE foo WITH CREATEROLE
 ALTER ROLE 'foo' WITH CREATEROLE -- normalized!
 ALTER ROLE ('foo') WITH CREATEROLE -- fully parenthetized
 ALTER ROLE _ WITH CREATEROLE -- literals removed
+ALTER ROLE '_' WITH CREATEROLE -- UNEXPECTED REPARSED AST WITHOUT LITERALS
 ALTER ROLE 'foo' WITH CREATEROLE -- identifiers removed
 
 parse
@@ -54,6 +60,7 @@ ALTER ROLE foo CREATEROLE
 ALTER ROLE 'foo' WITH CREATEROLE -- normalized!
 ALTER ROLE ('foo') WITH CREATEROLE -- fully parenthetized
 ALTER ROLE _ WITH CREATEROLE -- literals removed
+ALTER ROLE '_' WITH CREATEROLE -- UNEXPECTED REPARSED AST WITHOUT LITERALS
 ALTER ROLE 'foo' WITH CREATEROLE -- identifiers removed
 
 parse
@@ -62,6 +69,7 @@ ALTER ROLE foo CREATELOGIN
 ALTER ROLE 'foo' WITH CREATELOGIN -- normalized!
 ALTER ROLE ('foo') WITH CREATELOGIN -- fully parenthetized
 ALTER ROLE _ WITH CREATELOGIN -- literals removed
+ALTER ROLE '_' WITH CREATELOGIN -- UNEXPECTED REPARSED AST WITHOUT LITERALS
 ALTER ROLE 'foo' WITH CREATELOGIN -- identifiers removed
 
 parse
@@ -70,4 +78,5 @@ ALTER ROLE foo NOCREATELOGIN
 ALTER ROLE 'foo' WITH NOCREATELOGIN -- normalized!
 ALTER ROLE ('foo') WITH NOCREATELOGIN -- fully parenthetized
 ALTER ROLE _ WITH NOCREATELOGIN -- literals removed
+ALTER ROLE '_' WITH NOCREATELOGIN -- UNEXPECTED REPARSED AST WITHOUT LITERALS
 ALTER ROLE 'foo' WITH NOCREATELOGIN -- identifiers removed