Correctly bind and scope named window parameters in transformer (fixes …

…#546)

src/include/duckdb/parser/transformer.hpp

@@ -26,12 +26,29 @@ struct OrderByNode;
 //! parser representation into the DuckDB representation
 class Transformer {
 public:
+	Transformer(Transformer *parent = nullptr) : parent(parent) {}
+
 	//! Transforms a Postgres parse tree into a set of SQL Statements
 	bool TransformParseTree(PGList *tree, vector<unique_ptr<SQLStatement>> &statements);
 	string NodetypeToString(PGNodeTag type);
 
+	idx_t ParamCount() {
+		return parent ? parent->ParamCount() : prepared_statement_parameter_index;
+	}
+private:
+	Transformer *parent;
+	//! The current prepared statement parameter index
 	idx_t prepared_statement_parameter_index = 0;
+	//! Holds window expressions defined by name. We need those when transforming the expressions referring to them.
+	unordered_map<string, PGWindowDef *> window_clauses;
 
+	void SetParamCount(idx_t new_count) {
+		if (parent) {
+			parent->SetParamCount(new_count);
+		} else {
+			this->prepared_statement_parameter_index = new_count;
+		}
+	}
 private:
 	//! Transforms a Postgres statement into a single SQL statement
 	unique_ptr<SQLStatement> TransformStatement(PGNode *stmt);
@@ -166,9 +183,6 @@ class Transformer {
 	bool TransformExpressionList(PGList *list, vector<unique_ptr<ParsedExpression>> &result);
 
 	void TransformWindowDef(PGWindowDef *window_spec, WindowExpression *expr);
-
-	//! Holds window expressions defined by name. We need those when transforming the expressions referring to them.
-	unordered_map<string, PGWindowDef *> window_clauses;
 };
 
 } // namespace duckdb

src/parser/parser.cpp

@@ -34,7 +34,7 @@ void Parser::ParseQuery(string query) {
 	// SQLStatements
 	Transformer transformer;
 	transformer.TransformParseTree(parser.parse_tree, statements);
-	n_prepared_parameters = transformer.prepared_statement_parameter_index;
+	n_prepared_parameters = transformer.ParamCount();
 
 	if (statements.size() > 0) {
 		auto &last_statement = statements.back();

src/parser/transform/expression/transform_param_ref.cpp

@@ -10,10 +10,10 @@ unique_ptr<ParsedExpression> Transformer::TransformParamRef(PGParamRef *node) {
 	}
 	auto expr = make_unique<ParameterExpression>();
 	if (node->number == 0) {
-		expr->parameter_nr = prepared_statement_parameter_index + 1;
+		expr->parameter_nr = ParamCount() + 1;
 	} else {
 		expr->parameter_nr = node->number;
 	}
-	prepared_statement_parameter_index = max(prepared_statement_parameter_index, expr->parameter_nr);
+	SetParamCount(max(ParamCount(), expr->parameter_nr));
 	return move(expr);
 }

src/parser/transform/statement/transform_prepare.cpp

@@ -17,7 +17,7 @@ unique_ptr<PrepareStatement> Transformer::TransformPrepare(PGNode *node) {
 	auto result = make_unique<PrepareStatement>();
 	result->name = string(stmt->name);
 	result->statement = TransformStatement(stmt->query);
-	prepared_statement_parameter_index = 0;
+	SetParamCount(0);
 
 	return result;
 }

src/parser/transform/statement/transform_select.cpp

@@ -9,21 +9,6 @@ unique_ptr<SelectStatement> Transformer::TransformSelect(PGNode *node) {
 	auto stmt = reinterpret_cast<PGSelectStmt *>(node);
 	auto result = make_unique<SelectStatement>();
 
-	if (stmt->windowClause) {
-		for (auto window_ele = stmt->windowClause->head; window_ele != NULL; window_ele = window_ele->next) {
-			auto window_def = reinterpret_cast<PGWindowDef *>(window_ele->data.ptr_value);
-			assert(window_def);
-			assert(window_def->name);
-			auto window_name = StringUtil::Lower(string(window_def->name));
-
-			auto it = window_clauses.find(window_name);
-			if (it != window_clauses.end()) {
-				throw Exception("A window specification needs an unique name");
-			}
-			window_clauses[window_name] = window_def;
-		}
-	}
-
 	// may contain windows so second
 	if (stmt->withClause) {
 		TransformCTE(reinterpret_cast<PGWithClause *>(stmt->withClause), *result);

src/parser/transform/statement/transform_select_node.cpp

@@ -4,16 +4,34 @@
 #include "duckdb/parser/statement/select_statement.hpp"
 #include "duckdb/parser/transformer.hpp"
 #include "duckdb/parser/expression/star_expression.hpp"
+#include "duckdb/common/string_util.hpp"
 
 using namespace duckdb;
 using namespace std;
 
 unique_ptr<QueryNode> Transformer::TransformSelectNode(PGSelectStmt *stmt) {
 	unique_ptr<QueryNode> node;
+
 	switch (stmt->op) {
 	case PG_SETOP_NONE: {
 		node = make_unique<SelectNode>();
 		auto result = (SelectNode *)node.get();
+
+		if (stmt->windowClause) {
+			for (auto window_ele = stmt->windowClause->head; window_ele != NULL; window_ele = window_ele->next) {
+				auto window_def = reinterpret_cast<PGWindowDef *>(window_ele->data.ptr_value);
+				assert(window_def);
+				assert(window_def->name);
+				auto window_name = StringUtil::Lower(string(window_def->name));
+
+				auto it = window_clauses.find(window_name);
+				if (it != window_clauses.end()) {
+					throw Exception("A window specification needs an unique name");
+				}
+				window_clauses[window_name] = window_def;
+			}
+		}
+
 		// do this early so the value lists also have a `FROM`
 		if (stmt->valuesLists) {
 			// VALUES list, create an ExpressionList

src/parser/transform/tableref/transform_subquery.cpp

@@ -5,7 +5,8 @@ using namespace duckdb;
 using namespace std;
 
 unique_ptr<TableRef> Transformer::TransformRangeSubselect(PGRangeSubselect *root) {
-	auto subquery = TransformSelectNode((PGSelectStmt *)root->subquery);
+	Transformer subquery_transformer(this);
+	auto subquery = subquery_transformer.TransformSelectNode((PGSelectStmt *)root->subquery);
 	if (!subquery) {
 		return nullptr;
 	}

test/sql/simple/test_window.cpp

@@ -554,3 +554,42 @@ TEST_CASE("Test errors in binding window functions", "[window]") {
 	result = con.Query("SELECT MIN(i) OVER (PARTITION BY i ORDER BY i) FROM integers");
 	REQUIRE(CHECK_COLUMN(result, 0, {}));
 }
+
+TEST_CASE("Test binding of named window functions in CTEs", "[window]") {
+	unique_ptr<QueryResult> result;
+	DuckDB db(nullptr);
+	Connection con(db);
+	con.EnableQueryVerification();
+
+	// named window clause
+	result = con.Query("select i, lag(i) over named_window from (values (1), (2), (3)) as t (i) window named_window as (order by i);");
+	REQUIRE(CHECK_COLUMN(result, 0, {1, 2, 3}));
+	REQUIRE(CHECK_COLUMN(result, 1, {Value(), 1, 2}));
+
+	// named window clause in CTE
+	result = con.Query("with subquery as (select i, lag(i) over named_window from (values (1), (2), (3)) as t (i) window named_window as (order by i)) select * from subquery;");
+	REQUIRE(CHECK_COLUMN(result, 0, {1, 2, 3}));
+	REQUIRE(CHECK_COLUMN(result, 1, {Value(), 1, 2}));
+
+	// named window clause in subquery
+	result = con.Query("select * from (select i, lag(i) over named_window from (values (1), (2), (3)) as t (i) window named_window as (order by i)) t1;");
+	REQUIRE(CHECK_COLUMN(result, 0, {1, 2, 3}));
+	REQUIRE(CHECK_COLUMN(result, 1, {Value(), 1, 2}));
+
+	// named window clause in view
+	REQUIRE_NO_FAIL(con.Query("CREATE VIEW v1 AS select i, lag(i) over named_window from (values (1), (2), (3)) as t (i) window named_window as (order by i);"));
+
+	result = con.Query("select * from v1;");
+	REQUIRE(CHECK_COLUMN(result, 0, {1, 2, 3}));
+	REQUIRE(CHECK_COLUMN(result, 1, {Value(), 1, 2}));
+
+	// same window clause name multiple times but in different subqueries
+	result = con.Query("SELECT * FROM (SELECT i, lag(i) OVER named_window FROM ( VALUES (1), (2), (3)) AS t (i) window named_window AS ( ORDER BY i)) t1, (SELECT i, lag(i) OVER named_window FROM ( VALUES (1), (2), (3)) AS t (i) window named_window AS ( ORDER BY i)) t2 ORDER BY 1, 2, 3, 4;");
+	REQUIRE(CHECK_COLUMN(result, 0, {1, 1, 1, 2, 2, 2, 3, 3, 3}));
+	REQUIRE(CHECK_COLUMN(result, 1, {Value(), Value(), Value(), 1, 1, 1, 2, 2, 2}));
+	REQUIRE(CHECK_COLUMN(result, 2, {1, 2, 3, 1, 2, 3, 1, 2, 3}));
+	REQUIRE(CHECK_COLUMN(result, 3, {Value(), 1, 2, Value(), 1, 2, Value(), 1, 2}));
+
+	// we cannot use named window specifications of the main query inside CTEs
+	REQUIRE_FAIL(con.Query("WITH subquery AS (SELECT i, lag(i) OVER named_window FROM ( VALUES (1), (2), (3)) AS t (i)) SELECT * FROM subquery window named_window AS ( ORDER BY i);"));
+}