Skip to content

Commit

Permalink
Fix several CTE related issues
Browse files Browse the repository at this point in the history
Materialized CTEs did not work properly with INSERT statements

TransformRecursiveCTE was too restrictive

Materialized CTEs were sometimes inlined and thereby duplicated

Fix PIVOT statements after materialized CTE fixes
  • Loading branch information
kryonix committed Jul 11, 2024
1 parent fd883da commit f27a1f7
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 12 deletions.
9 changes: 3 additions & 6 deletions src/parser/transform/helpers/transform_cte.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,7 @@ unique_ptr<SelectStatement> Transformer::TransformRecursiveCTE(duckdb_libpgquery

unique_ptr<SelectStatement> select;
switch (stmt.op) {
case duckdb_libpgquery::PG_SETOP_UNION:
case duckdb_libpgquery::PG_SETOP_EXCEPT:
case duckdb_libpgquery::PG_SETOP_INTERSECT: {
case duckdb_libpgquery::PG_SETOP_UNION: {
select = make_uniq<SelectStatement>();
select->node = make_uniq_base<QueryNode, RecursiveCTENode>();
auto &result = select->node->Cast<RecursiveCTENode>();
Expand All @@ -111,11 +109,10 @@ unique_ptr<SelectStatement> Transformer::TransformRecursiveCTE(duckdb_libpgquery
result.left = TransformSelectNode(*PGPointerCast<duckdb_libpgquery::PGSelectStmt>(stmt.larg));
result.right = TransformSelectNode(*PGPointerCast<duckdb_libpgquery::PGSelectStmt>(stmt.rarg));
result.aliases = info.aliases;
if (stmt.op != duckdb_libpgquery::PG_SETOP_UNION) {
throw ParserException("Unsupported setop type for recursive CTE: only UNION or UNION ALL are supported");
}
break;
}
case duckdb_libpgquery::PG_SETOP_EXCEPT:
case duckdb_libpgquery::PG_SETOP_INTERSECT:
default:
// This CTE is not recursive. Fallback to regular query transformation.
return TransformSelect(*PGPointerCast<duckdb_libpgquery::PGSelectStmt>(cte.ctequery));
Expand Down
1 change: 0 additions & 1 deletion src/parser/transform/statement/transform_insert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ unique_ptr<InsertStatement> Transformer::TransformInsert(duckdb_libpgquery::PGIn
}
if (stmt.selectStmt) {
result->select_statement = TransformSelect(stmt.selectStmt, false);
result->select_statement->node = TransformMaterializedCTE(std::move(result->select_statement->node));
} else {
result->default_values = true;
}
Expand Down
2 changes: 1 addition & 1 deletion src/parser/transform/statement/transform_pivot_stmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ unique_ptr<SQLStatement> Transformer::GenerateCreateEnumStmt(unique_ptr<CreatePi
}

auto select = make_uniq<SelectStatement>();
select->node = std::move(subselect);
select->node = TransformMaterializedCTE(std::move(subselect));
info->query = std::move(select);
info->type = LogicalType::INVALID;

Expand Down
2 changes: 1 addition & 1 deletion src/planner/binder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ BoundStatement Binder::Bind(SQLStatement &statement) {
case StatementType::SELECT_STATEMENT:
return Bind(statement.Cast<SelectStatement>());
case StatementType::INSERT_STATEMENT:
return Bind(statement.Cast<InsertStatement>());
return BindWithCTE(statement.Cast<InsertStatement>());
case StatementType::COPY_STATEMENT:
return Bind(statement.Cast<CopyStatement>(), CopyToType::COPY_TO_FILE);
case StatementType::DELETE_STATEMENT:
Expand Down
9 changes: 9 additions & 0 deletions src/planner/binder/tableref/bind_basetableref.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,15 @@ unique_ptr<BoundTableRef> Binder::Bind(BaseTableRef &ref) {
// retry with next candidate CTE
continue;
}

// If we have found a materialized CTE, but no corresponding CTE binding,
// something is wrong.
if (cte.materialized == CTEMaterialize::CTE_MATERIALIZE_ALWAYS) {
throw BinderException(
"There is a WITH item named \"%s\", but it cannot be references from this part of the query.",
ref.table_name);
}

// Move CTE to subquery and bind recursively
SubqueryRef subquery(unique_ptr_cast<SQLStatement, SelectStatement>(cte.query->Copy()));
subquery.alias = ref.alias.empty() ? ref.table_name : ref.alias;
Expand Down
4 changes: 1 addition & 3 deletions test/sql/cte/materialized/test_cte_materialized.test
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,9 @@ with cte1 as MATERIALIZED (select 42), cte1 as MATERIALIZED (select 42) select *
----

# reference to CTE before its actually defined
query I
statement error
with cte3 as MATERIALIZED (select ref2.j as i from cte1 as ref2), cte1 as MATERIALIZED (Select i as j from a), cte2 as MATERIALIZED (select ref.j+1 as k from cte1 as ref) select * from cte2 union all select * FROM cte3;
----
43
42

# multiple uses of same CTE
query II
Expand Down
9 changes: 9 additions & 0 deletions test/sql/cte/materialized/test_materialized_cte.test
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,12 @@ WITH t(x) AS MATERIALIZED (SELECT 1),
----
2
1

statement error
WITH t0(x) AS MATERIALIZED (
SELECT x FROM t1
), t1(x) AS MATERIALIZED (
SELECT 1
)
SELECT * FROM t0;
----
41 changes: 41 additions & 0 deletions test/sql/cte/test_cte.test
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,44 @@ from cte
where alias2 > 0;
----
1 1

# recursive CTE and a non-recursive CTE with except
query I
WITH RECURSIVE t AS (
SELECT 1 AS a
UNION ALL
SELECT a+1
FROM t
WHERE a < 10
), s AS (
(VALUES (5), (6), (7), (8), (9), (10), (11), (12), (13), (42))
EXCEPT
TABLE t
)
SELECT * FROM s AS _(x) ORDER BY x;
----
11
12
13
42

# recursive CTE with except in recursive part (but not as a recursive anchor)
query I
WITH RECURSIVE t AS (
select 1 as a
union all
(select a+1
from t
where a < 10
except
SELECT 4)
), s AS (
(values (5), (6), (7))
except
table t
)
SELECT * FROM s AS _(x) ORDER BY x;
----
5
6
7

0 comments on commit f27a1f7

Please sign in to comment.