sql: validate recursive CTE references in subqueries and outer joins

pkg/sql/opt/optbuilder/builder.go

@@ -111,6 +111,10 @@ type Builder struct {
 	// Together, they form a directed acyclic graph.
 	cteRefMap map[opt.WithID]cteSources
 
+	// outerJoinNullExtendedDepth tracks the number of null-extended outer-join
+	// sides currently being built.
+	outerJoinNullExtendedDepth int
+
 	// If set, the planner will skip checking for the SELECT privilege when
 	// resolving data sources (tables, views, etc). This is used when compiling
 	// views and the view SELECT privilege has already been checked. This should
@@ -129,6 +133,10 @@ type Builder struct {
 	// (if any).
 	subquery *subquery
 
+	// subqueryDepth is the number of nested expression and FROM subqueries
+	// currently being built.
+	subqueryDepth int
+
 	// If set, we are processing a view definition; in this case, catalog caches
 	// are disabled and certain statements (like mutations) are disallowed.
 	insideViewDef bool

pkg/sql/opt/optbuilder/join.go

@@ -34,8 +34,13 @@ func (b *Builder) buildJoin(
 	if joinType == descpb.RightOuterJoin || joinType == descpb.FullOuterJoin {
 		leftLockCtx.isNullExtended = true
 	}
+	if joinType == descpb.RightOuterJoin || joinType == descpb.FullOuterJoin {
+		b.outerJoinNullExtendedDepth++
+	}
 	leftScope := b.buildDataSource(join.Left, nil /* indexFlags */, leftLockCtx, inScope)
-
+	if joinType == descpb.RightOuterJoin || joinType == descpb.FullOuterJoin {
+		b.outerJoinNullExtendedDepth--
+	}
 	inScopeRight := inScope
 	isLateral := b.exprIsLateral(join.Right)
 	if isLateral {
@@ -51,8 +56,13 @@ func (b *Builder) buildJoin(
 	if joinType == descpb.LeftOuterJoin || joinType == descpb.FullOuterJoin {
 		rightLockCtx.isNullExtended = true
 	}
+	if joinType == descpb.LeftOuterJoin || joinType == descpb.FullOuterJoin {
+		b.outerJoinNullExtendedDepth++
+	}
 	rightScope := b.buildDataSource(join.Right, nil /* indexFlags */, rightLockCtx, inScopeRight)
-
+	if joinType == descpb.LeftOuterJoin || joinType == descpb.FullOuterJoin {
+		b.outerJoinNullExtendedDepth--
+	}
 	// Check that the same table name is not used on both sides.
 	b.validateJoinTableNames(leftScope, rightScope)
 

pkg/sql/opt/optbuilder/select.go

@@ -194,7 +194,12 @@ func (b *Builder) buildDataSource(
 		// Postgres does, because then the AliasedTableExpr case would handle this.
 		lockCtx.withoutTargets()
 
-		outScope = b.buildSelectStmt(source.Select, lockCtx, nil /* desiredTypes */, inScope)
+		func() {
+			b.subqueryDepth++
+			defer func() { b.subqueryDepth-- }()
+
+			outScope = b.buildSelectStmt(source.Select, lockCtx, nil /* desiredTypes */, inScope)
+		}()
 
 		// Treat the subquery result as an anonymous data source (i.e. column names
 		// are not qualified). Remove hidden columns, as they are not accessible

pkg/sql/opt/optbuilder/subquery.go

@@ -216,6 +216,9 @@ func (s *subquery) buildSubquery(desiredTypes []*types.T) {
 	defer func() { s.scope.builder.subquery = outer }()
 	s.scope.builder.subquery = s
 
+	s.scope.builder.subqueryDepth++
+	defer func() { s.scope.builder.subqueryDepth-- }()
+
 	// We must push() here so that the columns in s.scope are correctly identified
 	// as outer columns.
 	outScope := s.scope.builder.buildStmt(s.Subquery.Select, desiredTypes, s.scope.push())

pkg/sql/opt/optbuilder/testdata/with

@@ -1222,6 +1222,63 @@ with &2 (cte)
            ├──  "?column?":7 => a:9
            └──  "?column?":8 => b:10
 
+# A recursive CTE inside an enclosing subquery is allowed when the recursive
+# reference itself does not appear inside another subquery.
+build
+SELECT (
+  WITH RECURSIVE cte(a) AS (
+      SELECT 1
+    UNION ALL
+      SELECT a + 1 FROM cte WHERE a < 3
+  )
+  SELECT max(a) FROM cte
+);
+----
+with &2 (cte)
+ ├── columns: max:7
+ ├── recursive-c-t-e
+ │    ├── columns: a:2
+ │    ├── working table binding: &1
+ │    ├── initial columns: "?column?":1
+ │    ├── recursive columns: "?column?":4
+ │    ├── fake-rel
+ │    │    └── columns: a:2
+ │    ├── project
+ │    │    ├── columns: "?column?":1!null
+ │    │    ├── values
+ │    │    │    └── ()
+ │    │    └── projections
+ │    │         └── 1 [as="?column?":1]
+ │    └── project
+ │         ├── columns: "?column?":4!null
+ │         ├── select
+ │         │    ├── columns: a:3!null
+ │         │    ├── with-scan &1 (cte)
+ │         │    │    ├── columns: a:3
+ │         │    │    └── mapping:
+ │         │    │         └──  a:2 => a:3
+ │         │    └── filters
+ │         │         └── a:3 < 3
+ │         └── projections
+ │              └── a:3 + 1 [as="?column?":4]
+ └── project
+      ├── columns: max:7
+      ├── values
+      │    └── ()
+      └── projections
+           └── subquery [as=max:7]
+                └── max1-row
+                     ├── columns: max:6
+                     └── scalar-group-by
+                          ├── columns: max:6
+                          ├── with-scan &2 (cte)
+                          │    ├── columns: a:5
+                          │    └── mapping:
+                          │         └──  a:2 => a:5
+                          └── aggregations
+                               └── max [as=max:6]
+                                    └── a:5
+
 # Error cases.
 build
 WITH RECURSIVE cte(a, b) AS (
@@ -1248,6 +1305,69 @@ WITH RECURSIVE cte(a, b) AS (
 ----
 error (42601): recursive reference to query "cte" must not appear more than once
 
+# Recursive references inside subqueries are not allowed by PostgreSQL.
+build
+WITH RECURSIVE cte(a, b) AS (
+    SELECT 1, 2
+  UNION ALL
+    SELECT 3, 4
+    WHERE 3 = (SELECT max(a) + 1 FROM cte)
+) SELECT * FROM cte;
+----
+error (42601): recursive reference to query "cte" must not appear within a subquery
+
+# Recursive references inside FROM subqueries are not allowed by PostgreSQL.
+build
+WITH RECURSIVE t(n) AS (
+    SELECT 1
+  UNION ALL
+    SELECT d.n + 1
+    FROM (SELECT n FROM t) AS d
+    WHERE d.n < 3
+)
+SELECT * FROM t;
+----
+error (42601): recursive reference to query "t" must not appear within a subquery
+
+# Recursive references on the nullable side of a LEFT JOIN are not allowed by PostgreSQL.
+build
+WITH RECURSIVE cte(a) AS (
+    SELECT 1
+  UNION ALL
+    SELECT cte.a + 1
+    FROM (VALUES (1)) AS v(x)
+    LEFT JOIN cte ON cte.a = v.x
+    WHERE cte.a < 10
+) SELECT * FROM cte;
+----
+error (42601): recursive reference to query "cte" must not appear within an outer join
+
+# Recursive references on the nullable side of a RIGHT JOIN are not allowed by PostgreSQL.
+build
+WITH RECURSIVE cte(a) AS (
+    SELECT 1
+  UNION ALL
+    SELECT cte.a + 1
+    FROM cte
+    RIGHT JOIN (VALUES (1)) AS v(x) ON cte.a = v.x
+    WHERE cte.a < 10
+) SELECT * FROM cte;
+----
+error (42601): recursive reference to query "cte" must not appear within an outer join
+
+# Recursive references on the nullable side of a FULL JOIN are not allowed by PostgreSQL.
+build
+WITH RECURSIVE cte(a) AS (
+    SELECT 1
+  UNION ALL
+    SELECT cte.a + 1
+    FROM cte
+    FULL JOIN (VALUES (1)) AS v(x) ON cte.a = v.x
+    WHERE cte.a < 10
+) SELECT * FROM cte;
+----
+error (42601): recursive reference to query "cte" must not appear within an outer join
+
 # If we really need to reference the working table multiple times, we can use
 # an inner WITH.
 build

pkg/sql/opt/optbuilder/with.go

@@ -290,11 +290,27 @@ func (b *Builder) buildCTE(
 
 	// We want to check if the recursive query is actually recursive. This is for
 	// annoying cases like `SELECT 1 UNION ALL SELECT 2`.
+	outerSubqueryDepth := b.subqueryDepth
+	outerJoinNullExtendedDepth := b.outerJoinNullExtendedDepth
+
 	numRefs := 0
 	cteSrc.onRef = func() {
+		if b.subqueryDepth != outerSubqueryDepth {
+			panic(pgerror.Newf(
+				pgcode.Syntax,
+				"recursive reference to query %q must not appear within a subquery",
+				cte.Name.Alias,
+			))
+		}
+		if b.outerJoinNullExtendedDepth > outerJoinNullExtendedDepth {
+			panic(pgerror.Newf(
+				pgcode.Syntax,
+				"recursive reference to query %q must not appear within an outer join",
+				cte.Name.Alias,
+			))
+		}
 		numRefs++
 	}
-
 	recursiveScope := b.buildStmt(recursive, initialTypes /* desiredTypes */, cteScope)
 	if numRefs == 0 {
 		// Build this as a non-recursive CTE.