Skip to content

Commit

Permalink
Merge pull request duckdb#9626 from Mytherin/pivotparameter
Browse files Browse the repository at this point in the history
Fix duckdb#9548 - Throw a more clear error when using parameters inside of the source of a top-level PIVOT statement
  • Loading branch information
Mytherin authored Nov 9, 2023
2 parents 1986284 + 7c7e2f2 commit ed80e70
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 21 deletions.
3 changes: 2 additions & 1 deletion src/include/duckdb/parser/transformer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class Transformer {
unique_ptr<SelectNode> base;
unique_ptr<ParsedExpression> column;
unique_ptr<QueryNode> subquery;
bool has_parameters;
};

public:
Expand Down Expand Up @@ -90,7 +91,7 @@ class Transformer {
bool GetParam(const string &name, idx_t &index, PreparedParamType type);

void AddPivotEntry(string enum_name, unique_ptr<SelectNode> source, unique_ptr<ParsedExpression> column,
unique_ptr<QueryNode> subquery);
unique_ptr<QueryNode> subquery, bool has_parameters);
unique_ptr<SQLStatement> GenerateCreateEnumStmt(unique_ptr<CreatePivotEntry> entry);
bool HasPivotEntries();
idx_t PivotEntryCount();
Expand Down
19 changes: 16 additions & 3 deletions src/parser/transform/statement/transform_pivot_stmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,18 @@
namespace duckdb {

void Transformer::AddPivotEntry(string enum_name, unique_ptr<SelectNode> base, unique_ptr<ParsedExpression> column,
unique_ptr<QueryNode> subquery) {
unique_ptr<QueryNode> subquery, bool has_parameters) {
if (parent) {
parent->AddPivotEntry(std::move(enum_name), std::move(base), std::move(column), std::move(subquery));
parent->AddPivotEntry(std::move(enum_name), std::move(base), std::move(column), std::move(subquery),
has_parameters);
return;
}
auto result = make_uniq<CreatePivotEntry>();
result->enum_name = std::move(enum_name);
result->base = std::move(base);
result->column = std::move(column);
result->subquery = std::move(subquery);
result->has_parameters = has_parameters;

pivot_entries.push_back(std::move(result));
}
Expand Down Expand Up @@ -113,6 +115,13 @@ unique_ptr<SQLStatement> Transformer::GenerateCreateEnumStmt(unique_ptr<CreatePi
unique_ptr<SQLStatement> Transformer::CreatePivotStatement(unique_ptr<SQLStatement> statement) {
auto result = make_uniq<MultiStatement>();
for (auto &pivot : pivot_entries) {
if (pivot->has_parameters) {
throw ParserException(
"PIVOT statements with pivot elements extracted from the data cannot have parameters in their source.\n"
"In order to use parameters the PIVOT values must be manually specified, e.g.:\n"
"PIVOT ... ON %s IN (val1, val2, ...)",
pivot->column->ToString());
}
result->statements.push_back(GenerateCreateEnumStmt(std::move(pivot)));
}
result->statements.push_back(std::move(statement));
Expand All @@ -125,7 +134,10 @@ unique_ptr<SQLStatement> Transformer::CreatePivotStatement(unique_ptr<SQLStateme

unique_ptr<QueryNode> Transformer::TransformPivotStatement(duckdb_libpgquery::PGSelectStmt &select) {
auto pivot = select.pivot;
auto current_param_count = ParamCount();
auto source = TransformTableRefNode(*pivot->source);
auto next_param_count = ParamCount();
bool has_parameters = next_param_count > current_param_count;

auto select_node = make_uniq<SelectNode>();
vector<unique_ptr<CTENode>> materialized_ctes;
Expand Down Expand Up @@ -171,7 +183,8 @@ unique_ptr<QueryNode> Transformer::TransformPivotStatement(duckdb_libpgquery::PG
auto new_select = make_uniq<SelectNode>();
ExtractCTEsRecursive(new_select->cte_map);
new_select->from_table = source->Copy();
AddPivotEntry(enum_name, std::move(new_select), col.pivot_expressions[0]->Copy(), std::move(col.subquery));
AddPivotEntry(enum_name, std::move(new_select), col.pivot_expressions[0]->Copy(), std::move(col.subquery),
has_parameters);
col.pivot_enum = enum_name;
}

Expand Down
52 changes: 35 additions & 17 deletions test/sql/pivot/pivot_prepare.test
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,27 @@ CREATE OR REPLACE TABLE monthly_sales(empid INT, amount INT, month TEXT);

statement ok
INSERT INTO monthly_sales VALUES
(1, 10000, 'JAN'),
(1, 400, 'JAN'),
(2, 4500, 'JAN'),
(2, 35000, 'JAN'),
(1, 5000, 'FEB'),
(1, 3000, 'FEB'),
(2, 200, 'FEB'),
(2, 90500, 'FEB'),
(1, 6000, 'MAR'),
(1, 5000, 'MAR'),
(2, 2500, 'MAR'),
(2, 9500, 'MAR'),
(1, 8000, 'APR'),
(1, 10000, 'APR'),
(2, 800, 'APR'),
(2, 4500, 'APR');
(1, 10000, '1-JAN'),
(1, 400, '1-JAN'),
(2, 4500, '1-JAN'),
(2, 35000, '1-JAN'),
(1, 5000, '2-FEB'),
(1, 3000, '2-FEB'),
(2, 200, '2-FEB'),
(2, 90500, '2-FEB'),
(1, 6000, '3-MAR'),
(1, 5000, '3-MAR'),
(2, 2500, '3-MAR'),
(2, 9500, '3-MAR'),
(1, 8000, '4-APR'),
(1, 10000, '4-APR'),
(2, 800, '4-APR'),
(2, 4500, '4-APR');

statement ok
PREPARE v1 AS SELECT *
FROM monthly_sales
PIVOT(SUM(amount + ?) FOR MONTH IN ('JAN', 'FEB', 'MAR', 'APR'))
PIVOT(SUM(amount + ?) FOR MONTH IN ('1-JAN', '2-FEB', '3-MAR', '4-APR'))
AS p
ORDER BY EMPID;

Expand All @@ -43,3 +43,21 @@ EXECUTE v1(1)
----
1 10402 8002 11002 18002
2 39502 90702 12002 5302

# prepare top-level pivot stmt
statement ok
PREPARE v2 AS
PIVOT monthly_sales ON MONTH USING SUM(AMOUNT + ?)

query IIIII rowsort
EXECUTE v2(1)
----
1 10402 8002 11002 18002
2 39502 90702 12002 5302

# parameters within subquery of top-level pivot statement not supported
statement error
PREPARE v3 AS
PIVOT (SELECT empid, amount + ? AS amount, month FROM monthly_sales) ON MONTH USING SUM(AMOUNT)
----
cannot have parameters in their source

0 comments on commit ed80e70

Please sign in to comment.