Merge cockroachdb#121871 cockroachdb#121872

121871: roachtest: ignore injected error in sqlsmith in some cases r=yuzefovich a=yuzefovich

We enable vectorized panic injection in `sqlsmith` roachtest, and we generally don't expect it to be propagated all the way as "internal error". However, sqlsmith itself issues some queries (like fetching all DB regions) which might hit this injected error which will result in a panic during `smither.Generate` call - we want to ignore such cases.

Fixes: cockroachdb#121467.

Release note: None

121872: sql: disallow unnamed INOUT parameters r=yuzefovich a=yuzefovich

We currently don't have an easy way to access the input / DEFAULT expression for the unnamed parameter, so we cannot populate the output correctly. For the time being, we'll disable INOUT unnamed parameters. Other parameter classes don't have this problem (unnamed IN parameters can only be referenced in the body via the $i notation, which we don't yet support; unnamed OUT parameters can also change their initial NULL value only via the $i notation).

Informs: cockroachdb#121251.

Epic: None

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>

pkg/ccl/logictestccl/testdata/logic_test/procedure_params

@@ -147,7 +147,7 @@ statement ok
 DROP PROCEDURE p;
 
 statement ok
-CREATE PROCEDURE p(IN INT, INOUT INT) AS $$ BEGIN END $$ LANGUAGE PLpgSQL;
+CREATE PROCEDURE p(IN INT, INOUT a INT) AS $$ BEGIN END $$ LANGUAGE PLpgSQL;
 
 # Argument expressions for IN and INOUT parameters are evaluated.
 statement error pgcode 22012 division by zero
@@ -216,7 +216,7 @@ statement error pgcode 42804 RETURN cannot have a parameter in function with OUT
 CREATE PROCEDURE p(OUT INT, OUT INT) AS $$ BEGIN RETURN NULL; END $$ LANGUAGE PLpgSQL;
 
 statement error pgcode 42804 RETURN cannot have a parameter in function with OUT parameters
-CREATE PROCEDURE p(INOUT INT) AS $$ BEGIN RETURN NULL; END $$ LANGUAGE PLpgSQL;
+CREATE PROCEDURE p(INOUT a INT) AS $$ BEGIN RETURN NULL; END $$ LANGUAGE PLpgSQL;
 
 statement ok
 CREATE PROCEDURE p(INOUT param1 INT, OUT param2 INT) AS $$
@@ -689,20 +689,21 @@ CALL my_sum(NULL, 1, 1);
 statement ok
 DROP PROCEDURE my_sum;
 
-statement ok
+statement error pgcode 0A000 unnamed INOUT parameters are not yet supported
 CREATE PROCEDURE my_sum(OUT a_plus_one INT, INOUT a INT, INOUT INT = 3) AS $$ BEGIN SELECT a + 1 INTO a_plus_one; END; $$ LANGUAGE PLpgSQL;
 
-# TODO(121251): this should return (2,1,3).
+statement ok
+CREATE PROCEDURE my_sum(OUT a_plus_one INT, INOUT a INT, INOUT b INT = 3) AS $$ BEGIN SELECT a + 1 INTO a_plus_one; END; $$ LANGUAGE PLpgSQL;
+
 query III
 CALL my_sum(NULL, 1);
 ----
-2  1  NULL
+2  1  3
 
-# TODO(121251): this should return (2,1,1).
 query III
 CALL my_sum(NULL, 1, 1);
 ----
-2  1  NULL
+2  1  1
 
 statement ok
 DROP PROCEDURE my_sum;

pkg/ccl/logictestccl/testdata/logic_test/procedure_plpgsql

@@ -605,20 +605,25 @@ SELECT * FROM temp;
 statement ok
 DROP PROCEDURE p(INOUT int);
 
-statement ok
+statement error pgcode 0A000 unnamed INOUT parameters are not yet supported
 CREATE PROCEDURE p(INOUT INT) AS $$
 BEGIN
   INSERT INTO temp VALUES(1);
   COMMIT;
 END; $$ LANGUAGE PLpgSQL;
 
-# TODO(121251): this should return 42 instead of NULL. We currently cannot
-# reference the unnamed parameter to get its input expression.
+statement ok
+CREATE PROCEDURE p(INOUT a INT) AS $$
+BEGIN
+  INSERT INTO temp VALUES(1);
+  COMMIT;
+END; $$ LANGUAGE PLpgSQL;
+
 query I colnames
 CALL p(42);
 ----
-column1
-NULL
+a
+42
 
 query I rowsort
 SELECT * FROM temp;

pkg/ccl/logictestccl/testdata/logic_test/udf_params

@@ -48,7 +48,7 @@ statement error RETURN cannot have a parameter in function with OUT parameters
 CREATE FUNCTION f(OUT INT, OUT INT) AS $$ BEGIN RETURN NULL; END $$ LANGUAGE PLpgSQL;
 
 statement error RETURN cannot have a parameter in function with OUT parameters
-CREATE FUNCTION f(INOUT INT) AS $$ BEGIN RETURN NULL; END $$ LANGUAGE PLpgSQL;
+CREATE FUNCTION f(INOUT a INT) AS $$ BEGIN RETURN NULL; END $$ LANGUAGE PLpgSQL;
 
 statement ok
 CREATE FUNCTION f(INOUT param1 INT, OUT param2 INT) RETURNS RECORD AS $$
@@ -850,33 +850,32 @@ SELECT * FROM my_sum(1, 1);
 statement ok
 DROP FUNCTION my_sum;
 
-statement ok
+statement error pgcode 0A000 unnamed INOUT parameters are not yet supported
 CREATE FUNCTION my_sum(OUT a_plus_one INT, INOUT a INT, INOUT INT = 3) AS $$ BEGIN SELECT a + 1 INTO a_plus_one; END; $$ LANGUAGE PLpgSQL;
 
-# TODO(121251): this should return (2,1,3).
+statement ok
+CREATE FUNCTION my_sum(OUT a_plus_one INT, INOUT a INT, INOUT b INT = 3) AS $$ BEGIN SELECT a + 1 INTO a_plus_one; END; $$ LANGUAGE PLpgSQL;
+
 query T
 SELECT my_sum(1);
 ----
-(2,1,)
+(2,1,3)
 
-# TODO(121251): this should return (2,1,3).
 query III colnames
 SELECT * FROM my_sum(1);
 ----
-a_plus_one  a  column3
-2           1  NULL
+a_plus_one  a  b
+2           1  3
 
-# TODO(121251): this should return (2,1,1).
 query T
 SELECT my_sum(1, 1);
 ----
-(2,1,)
+(2,1,1)
 
-# TODO(121251): this should return (2,1,3).
 query III
 SELECT * FROM my_sum(1, 1);
 ----
-2  1  NULL
+2  1  1
 
 statement ok
 DROP FUNCTION my_sum;

pkg/cmd/roachtest/tests/sqlsmith.go

@@ -261,27 +261,34 @@ WITH into_db = 'defaultdb', unsafe_restore_incompatible_version;
 				es := err.Error()
 				if strings.Contains(es, "internal error") {
 					var expectedError bool
-					switch {
-					case strings.Contains(es, "injected panic in optimizer"):
-						// Optimizer panic-injection surfaces as an internal
-						// error.
-						expectedError = true
-					case strings.Contains(es, "Failed generating a query") &&
-						strings.Contains(es, "injected panic in "):
-						// Vectorized panic was injected when sqlsmith itself
-						// issued a query to generate another query (for
-						// example, in getDatabaseRegions).
-						expectedError = true
+					for _, exp := range []string{
+						// Optimizer panic-injection surfaces as an internal error.
+						"injected panic in optimizer",
+					} {
+						expectedError = expectedError || strings.Contains(es, exp)
 					}
 					if !expectedError {
 						logStmt(stmt)
 						t.Fatalf("error: %s\nstmt:\n%s;", err, stmt)
 					}
-				} else if strings.Contains(es, "Empty statement returned by generate") ||
-					stmt == "" {
-					// Either were unable to generate a statement or
-					// we panicked making one.
-					t.Fatalf("Failed generating a query %s", err)
+				} else {
+					if strings.Contains(es, "Empty statement returned by generate") {
+						// We were unable to generate a statement - this is
+						// never expected.
+						t.Fatalf("Failed generating a query %s", err)
+					}
+					if stmt == "" {
+						// We panicked when generating a statement.
+						//
+						// The panic might be expected if it was a vectorized
+						// panic that was injected when sqlsmith itself issued a
+						// query to generate another query (for example, in
+						// getDatabaseRegions).
+						expectedError := strings.Contains(es, "injected panic in ")
+						if !expectedError {
+							t.Fatalf("Panicked when generating a query %s", err)
+						}
+					}
 				}
 				// Ignore other errors because they happen so
 				// frequently (due to sqlsmith not crafting

pkg/sql/logictest/testdata/logic_test/procedure_params

@@ -138,7 +138,7 @@ statement ok
 DROP PROCEDURE p;
 
 statement ok
-CREATE PROCEDURE p(IN INT, INOUT INT) AS $$ SELECT 1; $$ LANGUAGE SQL;
+CREATE PROCEDURE p(IN INT, INOUT a INT) AS $$ SELECT 1; $$ LANGUAGE SQL;
 
 # Argument expressions for IN and INOUT parameters are evaluated.
 statement error pgcode 22012 division by zero

pkg/sql/logictest/testdata/logic_test/udf_rewrite

@@ -108,7 +108,7 @@ statement ok
 DROP FUNCTION f_rewrite();
 
 statement ok
-CREATE FUNCTION f_rewrite(INOUT weekday) AS
+CREATE FUNCTION f_rewrite(INOUT a weekday) AS
 $$
   SELECT 'thursday'::weekday;
 $$ LANGUAGE SQL
@@ -168,7 +168,7 @@ statement ok
 DROP PROCEDURE p_rewrite();
 
 statement ok
-CREATE PROCEDURE p_rewrite(INOUT weekday) AS
+CREATE PROCEDURE p_rewrite(INOUT a weekday) AS
 $$
   SELECT 'thursday'::weekday;
 $$ LANGUAGE SQL

pkg/sql/opt/optbuilder/create_function.go

@@ -189,6 +189,9 @@ func (b *Builder) buildCreateFunction(cf *tree.CreateRoutine, inScope *scope) (o
 		if err != nil {
 			panic(err)
 		}
+		if param.Class == tree.RoutineParamInOut && param.Name == "" {
+			panic(unimplemented.NewWithIssue(121251, "unnamed INOUT parameters are not yet supported"))
+		}
 		if param.IsOutParam() {
 			outParamTypes = append(outParamTypes, typ)
 			paramName := string(param.Name)