diff --git a/pkg/sql/opt/optbuilder/builder.go b/pkg/sql/opt/optbuilder/builder.go index e1ad09df8271..52e1c86a3bf4 100644 --- a/pkg/sql/opt/optbuilder/builder.go +++ b/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 diff --git a/pkg/sql/opt/optbuilder/join.go b/pkg/sql/opt/optbuilder/join.go index aa409acf6f6a..32dd181a1b7e 100644 --- a/pkg/sql/opt/optbuilder/join.go +++ b/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) diff --git a/pkg/sql/opt/optbuilder/select.go b/pkg/sql/opt/optbuilder/select.go index 7b1125de6523..bd202f53b3f9 100644 --- a/pkg/sql/opt/optbuilder/select.go +++ b/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 diff --git a/pkg/sql/opt/optbuilder/subquery.go b/pkg/sql/opt/optbuilder/subquery.go index eec56f751e5a..02d480a42e9d 100644 --- a/pkg/sql/opt/optbuilder/subquery.go +++ b/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()) diff --git a/pkg/sql/opt/optbuilder/testdata/with b/pkg/sql/opt/optbuilder/testdata/with index efb745924dce..c71161a1c172 100644 --- a/pkg/sql/opt/optbuilder/testdata/with +++ b/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 diff --git a/pkg/sql/opt/optbuilder/with.go b/pkg/sql/opt/optbuilder/with.go index bb51ed963481..5051da89fac4 100644 --- a/pkg/sql/opt/optbuilder/with.go +++ b/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.