Merge #118569

118569: importer: disallow subqueries in function arguments for pg_dump r=DrewKimball a=DrewKimball

Some functions can modify the db schema, and so can be included in a pg_dump file. The pg_dump importer logic type-checks such functions, which can lead to a nil-pointer panic in cases like the following: `SELECT addgeometrycolumn('t', 'foo', 4326, (SELECT 'POINT'), 2)` This is because subqueries cannot be type-checked outside the optbuilder, which sets the type annotation for the subquery.

This patch explicitly disallows subqueries in the type-checking that happens for processing a pg_dump file. This will ensure that users get an expected `subqueries are not allowed in pg_dump function arguments` error, instead of the panic.

Fixes #117724

Release note (bug fix): Fixed a rare panic that could happen during a pg_dump import that contains a function like `SELECT addgeometrycolumn(...)`. Now, attempting to import a pg_dump with a function that has a subquery in one of its arguments results in an expected error.

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

pkg/sql/importer/read_import_pgdump.go

@@ -784,6 +784,7 @@ func readPostgresStmt(
 				case *tree.FuncExpr:
 					// Look for function calls that mutate schema (this is actually a thing).
 					semaCtx := tree.MakeSemaContext()
+					semaCtx.Properties.Require("pg_dump function arguments", tree.RejectSubqueries)
 					if _, err := expr.TypeCheck(ctx, &semaCtx, nil /* desired */); err != nil {
 						// If the expression does not type check, it may be a case of using
 						// a column that does not exist yet in a setval call (as is the case

pkg/sql/importer/read_import_pgdump_test.go

@@ -159,7 +159,7 @@ COPY done;
 	}
 }
 
-func TestImportArrayData(t *testing.T) {
+func TestImportPGDump(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 	ctx := context.Background()
@@ -184,6 +184,7 @@ func TestImportArrayData(t *testing.T) {
 		data        string
 		verifyQuery string
 		expected    [][]string
+		errorRE     string
 	}{
 		{
 			name: "text arrays",
@@ -211,7 +212,7 @@ COPY t (array_column) FROM STDIN;
 		},
 
 		{
-			name: "multiple columns",
+			name: "multiple array columns",
 			data: `
 CREATE TABLE t (
 	array_column int[],
@@ -226,6 +227,24 @@ COPY t (array_column, array_column2) FROM STDIN;
 				{"{1,2,3}", "{cat,dog}"},
 			},
 		},
+
+		{
+			name: "function",
+			data: `
+CREATE TABLE t (x INT);
+
+SELECT addgeometrycolumn('t', 'foo', 4326, 'POINT', 2)`,
+			expected: [][]string{},
+		},
+
+		{
+			name: "function with subquery",
+			data: `
+CREATE TABLE t (x INT);
+
+SELECT addgeometrycolumn('t', 'foo', 4326, (SELECT 'POINT'), 2)`,
+			errorRE: ".*subqueries are not allowed in pg_dump function arguments.*",
+		},
 	}
 
 	for _, test := range tests {
@@ -237,8 +256,12 @@ COPY t (array_column, array_column2) FROM STDIN;
 			data = test.data
 
 			// Import PGDump and verify expected behavior.
-			sqlDB.Exec(t, importDumpQuery, srv.URL)
-			sqlDB.CheckQueryResults(t, test.verifyQuery, test.expected)
+			if test.errorRE != "" {
+				sqlDB.ExpectErr(t, test.errorRE, importDumpQuery, srv.URL)
+			} else {
+				sqlDB.Exec(t, importDumpQuery, srv.URL)
+				sqlDB.CheckQueryResults(t, test.verifyQuery, test.expected)
+			}
 		})
 	}
 }