Merge #119034 #119507

119034: plpgsql: misc fixes for parsing and formatting identifiers and constants r=DrewKimball a=DrewKimball

#### plpgsql: correctly format variable names in declarations

This commit fixes formatting for variable and cursor declarations to
use `FmtCtx.FormatNode` instead of `FormatString`. This ensures that
variable names are correctly redacted when applicable, and that
complex identifiers round-trip through the parser.

Informs #106368

Release note (bug fix): Fixed a bug that prevented the use of PL/pgSQL
routines with complex variable names that require double quotes. This
bug has existed since v23.2.

#### plpgsql/parser: propagate errors during string scanning

This commit fixes handling for string constants in the PL/pgSQL parser.
Now, the PL/pgSQL scanner handles string scanning itself instead of delegating
to the SQL scanner. This ensures that the correct token ID is used when an
error occurs (e.g. unterminated string). This commit also adds a check for the
ERROR token during PL/pgSQL lexing, so that a scanning error is noticed and
propagated back to the user.

Informs #106368

Release note (bug fix): Fixed a bug that could cause creation of a
syntactically invalid PL/pgSQL routine to return the wrong error. This bug has
existed since v23.2.

#### plpgsql/parser: fix parsing for escaped strings

This commit adds support for scanning an escaped string constant in a
PL/pgSQL routine. This is necessary because escaped strings have imbalanced
quotes (like `e'foobar''`), which previously caused the scanner to return
an "unterminated string" error.

Informs #106368

Release note (bug fix): Fixed a bug that could result in a syntax error
if a PL/pgSQL routine was created with an escaped string constant in
the routine body. This bug has existed since v23.2.

119507: sql: normalize results for float aggregate functions r=DrewKimball a=DrewKimball

This commit normalizes the results of the float aggregates that should return return a non-negative result, to account for floating-point errors. This should decreases the rate of randomized test failures due to this problem.

Fixes #118733

Release note: None

Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>

pkg/sql/parser/parse_test.go

@@ -36,7 +36,7 @@ import (
 
 // TestParseDataDriven verifies that we can parse the supplied SQL and regenerate the SQL
 // string from the syntax tree.
-func TestParseDatadriven(t *testing.T) {
+func TestParseDataDriven(t *testing.T) {
 	datadriven.Walk(t, datapathutils.TestDataPath(t), func(t *testing.T, path string) {
 		datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string {
 			switch d.Cmd {

pkg/sql/plpgsql/parser/lexer.go

@@ -268,6 +268,11 @@ func (l *lexer) readSQLConstruct(
 	startPos = l.lastPos + 1
 	for l.lastPos < len(l.tokens) {
 		tok := l.Peek()
+		if tok.id == ERROR {
+			// This is a tokenizer (lexical) error: the scanner
+			// will have stored the error message in the string field.
+			return 0, 0, 0, errors.Newf("%s", tok.str)
+		}
 		if int(tok.id) == terminator1 && parenLevel == 0 {
 			terminatorMet = terminator1
 			break

pkg/sql/plpgsql/parser/plpgsql.y

@@ -662,7 +662,9 @@ proc_stmt:pl_block ';'
     $$.val = $1.statement()
   }
 | stmt_while
-  { }
+  {
+    $$.val = $1.statement()
+  }
 | stmt_for
   { }
 | stmt_foreach_a

pkg/sql/plpgsql/parser/testdata/combo

@@ -78,9 +78,9 @@ RETURN res;
 END;
  -- literals removed
 DECLARE
-i INT8;
-n INT8 := _(_, 1);
-res INT8[] := ARRAY[]::INT8[];
+_ INT8;
+_ INT8 := _(_, 1);
+_ INT8[] := ARRAY[]::INT8[];
 BEGIN
 _ := 0;
 LOOP

pkg/sql/plpgsql/parser/testdata/const

@@ -0,0 +1,153 @@
+parse
+DECLARE
+  x STRING := e'1GW\'';
+BEGIN
+  INSERT INTO tab SELECT e'1GW\'';
+END
+----
+DECLARE
+x STRING := e'1GW\'';
+BEGIN
+INSERT INTO tab SELECT e'1GW\'';
+END;
+ -- normalized!
+DECLARE
+x STRING := (e'1GW\'');
+BEGIN
+INSERT INTO tab SELECT (e'1GW\'');
+END;
+ -- fully parenthesized
+DECLARE
+x STRING := '_';
+BEGIN
+INSERT INTO tab SELECT '_';
+END;
+ -- literals removed
+DECLARE
+_ STRING := e'1GW\'';
+BEGIN
+INSERT INTO _ SELECT e'1GW\'';
+END;
+ -- identifiers removed
+
+parse
+DECLARE
+  x STRING := b'1GW\'';
+BEGIN
+  INSERT INTO tab SELECT b'1GW\'';
+END
+----
+DECLARE
+x STRING := b'1GW\'';
+BEGIN
+INSERT INTO tab SELECT b'1GW\'';
+END;
+ -- normalized!
+DECLARE
+x STRING := (b'1GW\'');
+BEGIN
+INSERT INTO tab SELECT (b'1GW\'');
+END;
+ -- fully parenthesized
+DECLARE
+x STRING := '_';
+BEGIN
+INSERT INTO tab SELECT '_';
+END;
+ -- literals removed
+DECLARE
+_ STRING := b'1GW\'';
+BEGIN
+INSERT INTO _ SELECT b'1GW\'';
+END;
+ -- identifiers removed
+
+parse
+DECLARE
+  x STRING := x'deadbeef';
+BEGIN
+  INSERT INTO tab SELECT x'deadbeef';
+END
+----
+DECLARE
+x STRING := b'\xde\xad\xbe\xef';
+BEGIN
+INSERT INTO tab SELECT b'\xde\xad\xbe\xef';
+END;
+ -- normalized!
+DECLARE
+x STRING := (b'\xde\xad\xbe\xef');
+BEGIN
+INSERT INTO tab SELECT (b'\xde\xad\xbe\xef');
+END;
+ -- fully parenthesized
+DECLARE
+x STRING := '_';
+BEGIN
+INSERT INTO tab SELECT '_';
+END;
+ -- literals removed
+DECLARE
+_ STRING := b'\xde\xad\xbe\xef';
+BEGIN
+INSERT INTO _ SELECT b'\xde\xad\xbe\xef';
+END;
+ -- identifiers removed
+
+parse
+DECLARE
+  x STRING := B'010101';
+BEGIN
+  INSERT INTO tab SELECT B'010101';
+END
+----
+DECLARE
+x STRING := B'010101';
+BEGIN
+INSERT INTO tab SELECT B'010101';
+END;
+ -- normalized!
+DECLARE
+x STRING := (B'010101');
+BEGIN
+INSERT INTO tab SELECT (B'010101');
+END;
+ -- fully parenthesized
+DECLARE
+x STRING := _;
+BEGIN
+INSERT INTO tab SELECT _;
+END;
+ -- literals removed
+DECLARE
+_ STRING := B'010101';
+BEGIN
+INSERT INTO _ SELECT B'010101';
+END;
+ -- identifiers removed
+
+error
+DECLARE
+  x STRING := 'foo;
+BEGIN
+  RETURN x;
+END
+----
+at or near ":": syntax error: unterminated string
+DETAIL: source SQL:
+DECLARE
+  x STRING := 'foo;
+           ^
+
+error
+DECLARE
+  x STRING := $foo$ foo;
+BEGIN
+  RETURN x;
+END
+----
+at or near ":": syntax error: unterminated string
+DETAIL: source SQL:
+DECLARE
+  x STRING := $foo$ foo;
+           ^

pkg/sql/plpgsql/parser/testdata/decl_header

@@ -25,7 +25,7 @@ END foo;
  -- literals removed
 <<_>>
 DECLARE
-var1 INT8 := 30;
+_ INT8 := 30;
 BEGIN
 END _;
  -- identifiers removed
@@ -57,7 +57,7 @@ END foo;
  -- literals removed
 <<_>>
 DECLARE
-var1 CONSTANT INT8 COLLATE _ NOT NULL := 30;
+_ CONSTANT INT8 COLLATE _ NOT NULL := 30;
 BEGIN
 END _;
  -- identifiers removed
@@ -84,7 +84,34 @@ BEGIN
 END;
  -- literals removed
 DECLARE
-var1 CONSTANT INT8 COLLATE _ NOT NULL := 30;
+_ CONSTANT INT8 COLLATE _ NOT NULL := 30;
+BEGIN
+END;
+ -- identifiers removed
+
+parse
+DECLARE
+  "%_2jd9$f foo" integer := 30;
+BEGIN
+END
+----
+DECLARE
+"%_2jd9$f foo" INT8 := 30;
+BEGIN
+END;
+ -- normalized!
+DECLARE
+"%_2jd9$f foo" INT8 := (30);
+BEGIN
+END;
+ -- fully parenthesized
+DECLARE
+"%_2jd9$f foo" INT8 := _;
+BEGIN
+END;
+ -- literals removed
+DECLARE
+_ INT8 := 30;
 BEGIN
 END;
  -- identifiers removed
@@ -138,7 +165,7 @@ BEGIN
 END;
  -- literals removed
 DECLARE
-var1 CURSOR FOR SELECT * FROM _ WHERE _ = _;
+_ CURSOR FOR SELECT * FROM _ WHERE _ = _;
 BEGIN
 END;
  -- identifiers removed

pkg/sql/plpgsql/parser/testdata/stmt_assign

@@ -57,6 +57,38 @@ _ := NULL;
 END;
  -- identifiers removed
 
+parse
+DECLARE
+  "%_2jd9$f foo" integer := 30;
+BEGIN
+  "%_2jd9$f foo" := 100;
+END
+----
+DECLARE
+"%_2jd9$f foo" INT8 := 30;
+BEGIN
+"%_2jd9$f foo" := 100;
+END;
+ -- normalized!
+DECLARE
+"%_2jd9$f foo" INT8 := (30);
+BEGIN
+"%_2jd9$f foo" := (100);
+END;
+ -- fully parenthesized
+DECLARE
+"%_2jd9$f foo" INT8 := _;
+BEGIN
+"%_2jd9$f foo" := _;
+END;
+ -- literals removed
+DECLARE
+_ INT8 := 30;
+BEGIN
+_ := 100;
+END;
+ -- identifiers removed
+
 error
 DECLARE
 BEGIN

pkg/sql/plpgsql/parser/testdata/stmt_block

@@ -135,10 +135,10 @@ RAISE NOTICE '_', x;
 END;
  -- literals removed
 DECLARE
-x INT8;
+_ INT8;
 BEGIN
 DECLARE
-y INT8;
+_ INT8;
 BEGIN
 RAISE NOTICE '% %', _, _;
 END;
@@ -184,7 +184,7 @@ RAISE NOTICE '_', x;
 END;
  -- literals removed
 DECLARE
-x INT8;
+_ INT8;
 BEGIN
 BEGIN
 _ := 100;
@@ -243,7 +243,7 @@ RAISE NOTICE '_', x;
 END;
  -- literals removed
 DECLARE
-x INT8;
+_ INT8;
 BEGIN
 BEGIN
 _ := 1 // 0;

pkg/sql/plpgsql/parser/testdata/stmt_exec_sql

@@ -306,7 +306,7 @@ RETURN i;
 END;
  -- literals removed
 DECLARE
-i INT8;
+_ INT8;
 BEGIN
 SET testing_optimizer_disable_rule_probability = 0;
 INSERT INTO _ VALUES (1, 2);

pkg/sql/plpgsql/parser/testdata/stmt_raise

@@ -198,7 +198,7 @@ RAISE '_', i;
 END;
  -- literals removed
 DECLARE
-i INT8 := 0;
+_ INT8 := 0;
 BEGIN
 RAISE 'foo %', _;
 END;
@@ -230,7 +230,7 @@ RAISE '_', i, i * _, i * _;
 END;
  -- literals removed
 DECLARE
-i INT8 := 0;
+_ INT8 := 0;
 BEGIN
 RAISE 'foo %, %, %.', _, _ * 2, _ * 100;
 END;
@@ -262,7 +262,7 @@ RAISE '_', (SELECT count(*) FROM xy WHERE x = i);
 END;
  -- literals removed
 DECLARE
-i INT8 := 0;
+_ INT8 := 0;
 BEGIN
 RAISE 'foo %', (SELECT _(*) FROM _ WHERE _ = _);
 END;