diff --git a/pkg/sql/opt/optbuilder/builder.go b/pkg/sql/opt/optbuilder/builder.go index e1ad09df8271..8365afb630b4 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 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/testdata/with b/pkg/sql/opt/optbuilder/testdata/with index efb745924dce..bf429d7347ce 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,56 @@ 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 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..3e61ad8ad3ef 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`. + outerSubquery := b.subquery + outerJoinNullExtendedDepth := b.outerJoinNullExtendedDepth + numRefs := 0 cteSrc.onRef = func() { + if b.subquery != outerSubquery { + 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.