Skip to content

Commit

Permalink
opt: better error message for unsupported UNION variant of recursive …
Browse files Browse the repository at this point in the history
…CTEs

Postgres supports a variant of recursive CTEs that uses UNION instead
of UNION ALL. We don't support it as of yet; this commit improves the
error returned in this case.

Fixes #46378.

Release note: None

Release justification: low risk change to new functionality.
  • Loading branch information
RaduBerinde committed Mar 27, 2020
1 parent d445101 commit 407cb4c
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 15 deletions.
36 changes: 35 additions & 1 deletion pkg/sql/opt/optbuilder/testdata/with
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,40 @@ with &2 (cte)
├── "?column?":7 => a:9
└── "?column?":8 => b:10

# Allow non-recursive CTE even when it has UNION.
build
WITH RECURSIVE cte(a, b) AS (
SELECT 1, 2
UNION
SELECT 3, 4
) SELECT * FROM cte;
----
with &2 (cte)
├── columns: a:7!null b:8!null
├── union
│ ├── columns: "?column?":5!null "?column?":6!null
│ ├── left columns: "?column?":1 "?column?":2
│ ├── right columns: "?column?":3 "?column?":4
│ ├── project
│ │ ├── columns: "?column?":1!null "?column?":2!null
│ │ ├── values
│ │ │ └── ()
│ │ └── projections
│ │ ├── 1 [as="?column?":1]
│ │ └── 2 [as="?column?":2]
│ └── project
│ ├── columns: "?column?":3!null "?column?":4!null
│ ├── values
│ │ └── ()
│ └── projections
│ ├── 3 [as="?column?":3]
│ └── 4 [as="?column?":4]
└── with-scan &2 (cte)
├── columns: a:7!null b:8!null
└── mapping:
├── "?column?":5 => a:7
└── "?column?":6 => b:8

# Error cases.
build
WITH RECURSIVE cte(a, b) AS (
Expand All @@ -1158,7 +1192,7 @@ WITH RECURSIVE cte(a, b) AS (
SELECT 1+a, 1+b FROM cte
) SELECT * FROM cte;
----
error (42601): recursive query "cte" does not have the form non-recursive-term UNION ALL recursive-term
error (0A000): unimplemented: recursive query "cte" uses UNION which is not implemented (only UNION ALL is supported)

build
WITH RECURSIVE cte(a, b) AS (
Expand Down
33 changes: 21 additions & 12 deletions pkg/sql/opt/optbuilder/with.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,25 @@ func (b *Builder) buildCTE(
}
cteScope.ctes = map[string]*cteSource{cte.Name.Alias.String(): cteSrc}

initial, recursive, ok := b.splitRecursiveCTE(cte.Stmt)
if !ok {
initial, recursive, isUnionAll, ok := b.splitRecursiveCTE(cte.Stmt)
// We don't currently support the UNION form (only UNION ALL).
if !ok || !isUnionAll {
// Build this as a non-recursive CTE, but throw a proper error message if it
// does have a recursive reference.
cteSrc.onRef = func() {
panic(pgerror.Newf(
pgcode.Syntax,
"recursive query %q does not have the form non-recursive-term UNION ALL recursive-term",
cte.Name.Alias,
))
if !ok {
panic(pgerror.Newf(
pgcode.Syntax,
"recursive query %q does not have the form non-recursive-term UNION ALL recursive-term",
cte.Name.Alias,
))
} else {
panic(unimplementedWithIssueDetailf(
46642, "",
"recursive query %q uses UNION which is not implemented (only UNION ALL is supported)",
cte.Name.Alias,
))
}
}
return b.buildCTE(cte, cteScope, false /* recursive */)
}
Expand Down Expand Up @@ -228,16 +237,16 @@ func (b *Builder) getCTECols(cteScope *scope, name tree.AliasClause) physical.Pr
// returns ok=false.
func (b *Builder) splitRecursiveCTE(
stmt tree.Statement,
) (initial, recursive *tree.Select, ok bool) {
) (initial, recursive *tree.Select, isUnionAll bool, ok bool) {
sel, ok := stmt.(*tree.Select)
// The form above doesn't allow for "outer" WITH, ORDER BY, or LIMIT
// clauses.
if !ok || sel.With != nil || sel.OrderBy != nil || sel.Limit != nil {
return nil, nil, false
return nil, nil, false, false
}
union, ok := sel.Select.(*tree.UnionClause)
if !ok || union.Type != tree.UnionOp || !union.All {
return nil, nil, false
if !ok || union.Type != tree.UnionOp {
return nil, nil, false, false
}
return union.Left, union.Right, true
return union.Left, union.Right, union.All, true
}
3 changes: 1 addition & 2 deletions pkg/util/errorutil/unimplemented/unimplemented.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ func NewWithIssueDetail(issue int, detail, msg string) error {
return unimplementedInternal(1 /*depth*/, issue, detail, false /*format*/, msg)
}

// NewWithIssueDetailf is like UnimplementedWithIssueDetail
// but the message is formatted.
// NewWithIssueDetailf is like NewWithIssueDetail but the message is formatted.
func NewWithIssueDetailf(issue int, detail, format string, args ...interface{}) error {
return unimplementedInternal(1 /*depth*/, issue, detail, true /*format*/, format, args...)
}
Expand Down

0 comments on commit 407cb4c

Please sign in to comment.