sql/opt: support anonymous subqueries in CREATE FUNCTION

Release note (bug fix): CockroachDB now supports using subqueries in
UDFs without an AS clause, for consistency with the syntax supported
outside of UDFs. (Subqueries without an AS clause is a CRDB-specific
extension.)

pkg/ccl/backupccl/backup_test.go

@@ -5312,7 +5312,7 @@ func TestBackupRestoreSequencesInViews(t *testing.T) {
 		sqlDB.CheckQueryResults(t, `SELECT * FROM d.v`, [][]string{{"2"}})
 		sqlDB.CheckQueryResults(t, `SHOW CREATE VIEW d.v`, [][]string{{
 			"d.public.v", "CREATE VIEW public.v (\n\tk\n) AS " +
-				"SELECT k FROM (SELECT nextval('public.s2'::REGCLASS) AS k)",
+				"SELECT k FROM (SELECT nextval('public.s2'::REGCLASS) AS k) AS \"?subquery1?\"",
 		}})
 
 		// Test that references are still tracked.
@@ -5338,7 +5338,7 @@ func TestBackupRestoreSequencesInViews(t *testing.T) {
 		sqlDB.Exec(t, `RESTORE TABLE s, v FROM 'nodelocal://0/test/'`)
 		sqlDB.CheckQueryResults(t, `SHOW CREATE VIEW d.v`, [][]string{{
 			"d.public.v", "CREATE VIEW public.v (\n\tk\n) AS " +
-				"(SELECT k FROM (SELECT nextval('public.s'::REGCLASS) AS k))",
+				"(SELECT k FROM (SELECT nextval('public.s'::REGCLASS) AS k) AS \"?subquery1?\")",
 		}})
 
 		// Check that v is not corrupted.
@@ -5348,7 +5348,7 @@ func TestBackupRestoreSequencesInViews(t *testing.T) {
 		sqlDB.Exec(t, `ALTER SEQUENCE s RENAME TO s2`)
 		sqlDB.CheckQueryResults(t, `SHOW CREATE VIEW d.v`, [][]string{{
 			"d.public.v", "CREATE VIEW public.v (\n\tk\n) AS " +
-				"(SELECT k FROM (SELECT nextval('public.s2'::REGCLASS) AS k))",
+				"(SELECT k FROM (SELECT nextval('public.s2'::REGCLASS) AS k) AS \"?subquery1?\")",
 		}})
 		sqlDB.CheckQueryResults(t, `SELECT * FROM v`, [][]string{{"2"}})
 

pkg/sql/logictest/testdata/logic_test/sequences_regclass

@@ -430,7 +430,10 @@ SHOW CREATE VIEW v2
 ----
 v2  CREATE VIEW public.v2 (
       currval
-    ) AS SELECT currval FROM (SELECT currval('public.view_seq'::REGCLASS) FROM test.public.t3)
+    ) AS SELECT
+        currval
+      FROM
+        (SELECT currval('public.view_seq'::REGCLASS) FROM test.public.t3) AS "?subquery1?"
 
 # Union containing sequences.
 statement ok

pkg/sql/logictest/testdata/logic_test/udf_star

@@ -18,7 +18,7 @@ $$
   SELECT * FROM (SELECT a FROM (SELECT * FROM t_onecol) AS foo) AS bar;
 $$ LANGUAGE SQL;
 
-statement error pq: unimplemented: unaliased subquery inside a function definition
+statement ok
 CREATE FUNCTION f_subquery_unaliased() RETURNS INT AS
 $$
   SELECT * FROM (SELECT a FROM (SELECT * FROM t_onecol));
@@ -68,8 +68,8 @@ $$
   SELECT word FROM (SELECT (pg_get_keywords()).* ORDER BY word LIMIT 1) AS foo;
 $$ LANGUAGE SQL;
 
-statement error pq: unimplemented: unaliased subquery inside a function definition
-CREATE FUNCTION f_ambiguous() RETURNS INT AS
+statement ok
+CREATE FUNCTION f_anon_subqueries() RETURNS INT AS
 $$
   SELECT * FROM (SELECT a FROM t_onecol) JOIN (SELECT a FROM t_twocol) ON true;
   SELECT 1;
@@ -88,15 +88,18 @@ FROM pg_catalog.pg_proc WHERE proname LIKE 'f\_%' ORDER BY oid;
 ----
 100108  f_unqualified_onecol      SELECT t_onecol.a FROM test.public.t_onecol;
 100109  f_subquery                SELECT bar.a FROM (SELECT a FROM (SELECT t_onecol.a FROM test.public.t_onecol) AS foo) AS bar;
-100110  f_unqualified_twocol      SELECT t_twocol.a, t_twocol.b FROM test.public.t_twocol;
-100111  f_allcolsel               SELECT t_twocol.a, t_twocol.b FROM test.public.t_twocol;
-100112  f_allcolsel_alias         SELECT t1.a, t1.b FROM test.public.t_twocol AS t1, test.public.t_twocol AS t2 WHERE t1.a = t2.a;
-100113  f_tuplestar               SELECT t_twocol.a, t_twocol.b FROM test.public.t_twocol;
-100114  f_unqualified_multicol    SELECT t_onecol.a, a FROM test.public.t_onecol;
+100110  f_subquery_unaliased      SELECT "?subquery1?".a FROM (SELECT a FROM (SELECT t_onecol.a FROM test.public.t_onecol) AS "?subquery2?") AS "?subquery1?";
+100111  f_unqualified_twocol      SELECT t_twocol.a, t_twocol.b FROM test.public.t_twocol;
+100112  f_allcolsel               SELECT t_twocol.a, t_twocol.b FROM test.public.t_twocol;
+100113  f_allcolsel_alias         SELECT t1.a, t1.b FROM test.public.t_twocol AS t1, test.public.t_twocol AS t2 WHERE t1.a = t2.a;
+100114  f_tuplestar               SELECT t_twocol.a, t_twocol.b FROM test.public.t_twocol;
+100115  f_unqualified_multicol    SELECT t_onecol.a, a FROM test.public.t_onecol;
                                   SELECT 1;
-100115  f_unqualified_doublestar  SELECT t_onecol.a, t_onecol.a FROM test.public.t_onecol;
+100116  f_unqualified_doublestar  SELECT t_onecol.a, t_onecol.a FROM test.public.t_onecol;
+                                  SELECT 1;
+100117  f_exprstar                SELECT word FROM (SELECT (pg_get_keywords()).word, (pg_get_keywords()).catcode, (pg_get_keywords()).catdesc ORDER BY word LIMIT 1) AS foo;
+100118  f_anon_subqueries         SELECT "?subquery1?".a, "?subquery2?".a FROM (SELECT a FROM test.public.t_onecol) AS "?subquery1?" JOIN (SELECT a FROM test.public.t_twocol) AS "?subquery2?" ON true;
                                   SELECT 1;
-100116  f_exprstar                SELECT word FROM (SELECT (pg_get_keywords()).word, (pg_get_keywords()).catcode, (pg_get_keywords()).catdesc ORDER BY word LIMIT 1) AS foo;
 
 
 query TT
@@ -211,16 +214,31 @@ statement ok
 DROP FUNCTION f_tuplestar;
 DROP FUNCTION f_allcolsel_alias;
 
-# Dropping a column using CASCADE is ok.
+# Dropping a column using CASCADE is ok,
+# although the legacy schema changer has troubles with it,
+# see: https://github.com/cockroachdb/cockroach/issues/97546
+skipif config local-legacy-schema-changer
 statement ok
 ALTER TABLE t_twocol DROP COLUMN b CASCADE;
 
 statement ok
 DROP TABLE t_onecol CASCADE;
 
 # The only remaining function should not reference the tables.
+# NB: remove the skipif directive when #97546 is resolved.
+skipif config local-legacy-schema-changer
+query TTT
+SELECT oid, proname, prosrc
+FROM pg_catalog.pg_proc WHERE proname LIKE 'f\_%' ORDER BY oid;
+----
+100117  f_exprstar  SELECT word FROM (SELECT (pg_get_keywords()).word, (pg_get_keywords()).catcode, (pg_get_keywords()).catdesc ORDER BY word LIMIT 1) AS foo;
+
+# Remove this when #97546 is resolved.
+onlyif config local-legacy-schema-changer
 query TTT
 SELECT oid, proname, prosrc
 FROM pg_catalog.pg_proc WHERE proname LIKE 'f\_%' ORDER BY oid;
 ----
-100116  f_exprstar                SELECT word FROM (SELECT (pg_get_keywords()).word, (pg_get_keywords()).catcode, (pg_get_keywords()).catdesc ORDER BY word LIMIT 1) AS foo;
+100111  f_unqualified_twocol  SELECT t_twocol.a, t_twocol.b FROM test.public.t_twocol;
+100112  f_allcolsel           SELECT t_twocol.a, t_twocol.b FROM test.public.t_twocol;
+100117  f_exprstar            SELECT word FROM (SELECT (pg_get_keywords()).word, (pg_get_keywords()).catcode, (pg_get_keywords()).catdesc ORDER BY word LIMIT 1) AS foo;

pkg/sql/logictest/testdata/logic_test/views

@@ -1185,7 +1185,7 @@ SHOW CREATE VIEW v12
 ----
 v12  CREATE VIEW public.v12 (
        k
-     ) AS (SELECT k FROM (SELECT 'a':::db2.public.view_type_new AS k))
+     ) AS (SELECT k FROM (SELECT 'a':::db2.public.view_type_new AS k) AS "?subquery1?")
 
 query T
 SELECT * FROM v12

pkg/sql/opt/optbuilder/builder.go

@@ -153,6 +153,10 @@ type Builder struct {
 	// (without ON CONFLICT) or false otherwise. All mutated tables will have an
 	// entry in the map.
 	areAllTableMutationsSimpleInserts map[cat.StableID]bool
+
+	// subqueryNameIdx helps generate unique subquery names during star
+	// expansion.
+	subqueryNameIdx int
 }
 
 // New creates a new Builder structure initialized with the given

pkg/sql/opt/optbuilder/select.go

@@ -12,6 +12,7 @@ package optbuilder
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
@@ -58,16 +59,29 @@ func (b *Builder) buildDataSource(
 			telemetry.Inc(sqltelemetry.IndexHintSelectUseCounter)
 			indexFlags = source.IndexFlags
 		}
+
+		if source.As.Alias == "" {
+			// The alias is an empty string. If we are in a view or UDF
+			// definition, we are also expanding stars and for this we need
+			// to ensure all unnamed subqueries have a name. (unnamed
+			// subqueries are a CRDB extension, so the behavior in that case
+			// can be CRDB-specific.)
+			//
+			// We do not perform this name assignment in the common case
+			// (everything else besides CREATE VIEW/FUNCTION) so as to save
+			// the cost of the string alloc / name propagation.
+			if _, ok := source.Expr.(*tree.Subquery); ok && (b.insideFuncDef || b.insideViewDef) {
+				b.subqueryNameIdx++
+				// The structure of this name is analogous to the auto-generated
+				// names for anonymous scalar expressions.
+				source.As.Alias = tree.Name(fmt.Sprintf("?subquery%d?", b.subqueryNameIdx))
+			}
+		}
+
 		if source.As.Alias != "" {
 			inScope = inScope.push()
 			inScope.alias = &source.As
 			locking = locking.filter(source.As.Alias)
-		} else if b.insideFuncDef {
-			// TODO(96375): Allow non-aliased subexpressions in UDFs after ambiguous
-			// columns can be correctly identified.
-			if _, ok := source.Expr.(*tree.Subquery); ok {
-				panic(unimplemented.New("user-defined functions", "unaliased subquery inside a function definition"))
-			}
 		}
 
 		outScope = b.buildDataSource(source.Expr, indexFlags, locking, inScope)

pkg/sql/opt/optbuilder/testdata/create_function

@@ -118,6 +118,17 @@ create-function
  └── dependencies
       └── ab [columns: a b]
 
+build
+CREATE FUNCTION f() RETURNS ab LANGUAGE SQL AS $$ SELECT * FROM (SELECT * from ab) $$
+----
+create-function
+ ├── CREATE FUNCTION f()
+ │   	RETURNS ab
+ │   	LANGUAGE SQL
+ │   	AS $$SELECT "?subquery1?".a, "?subquery1?".b FROM (SELECT ab.a, ab.b FROM t.public.ab) AS "?subquery1?";$$
+ └── dependencies
+      └── ab [columns: a b]
+
 build
 CREATE FUNCTION f() RETURNS INT LANGUAGE SQL BEGIN ATOMIC SELECT 1; END;
 ----

pkg/sql/opt/optbuilder/testdata/create_view

@@ -258,7 +258,7 @@ build
 CREATE VIEW v16 AS SELECT a FROM (SELECT a,b FROM ab);
 ----
 create-view t.public.v16
- ├── SELECT a FROM (SELECT a, b FROM t.public.ab)
+ ├── SELECT a FROM (SELECT a, b FROM t.public.ab) AS "?subquery1?"
  ├── columns: a:1
  └── dependencies
       └── ab [columns: a b]

pkg/sql/table_test.go

@@ -586,7 +586,7 @@ func TestSerializedUDTsInView(t *testing.T) {
 		// Test when a UDT is used in various parts of a view (subquery, CTE, etc.).
 		{
 			"SELECT k FROM (SELECT 'hello'::greeting AS k)",
-			`(SELECT k FROM (SELECT b'\x80':::@$OID AS k))`,
+			`(SELECT k FROM (SELECT b'\x80':::@$OID AS k) AS "?subquery1?")`,
 		},
 		{
 			"WITH w AS (SELECT 'hello':::greeting AS k) SELECT k FROM w",