kosiew commented on code in PR #23789:
URL: https://github.com/apache/datafusion/pull/23789#discussion_r3636758212
##########
datafusion/core/tests/sql/unparser.rs:
##########
@@ -612,6 +664,87 @@ async fn
optimized_duckdb_unparse_qualify_unqualifies_agg_input() -> Result<()>
Ok(())
}
+#[tokio::test]
+async fn optimized_duckdb_unparse_window_over_agg_unqualifies_input() ->
Result<()> {
+ let ctx = issue_23317_context()?;
+ assert!(ctx.remove_optimizer_rule(SingleDistinctToGroupBy::new().name()));
+
+ let plan = ctx
+ .sql(ISSUE_23668_WINDOW_QUERY)
+ .await?
+ .into_optimized_plan()?;
+ let dialect = DuckDBDialect::new();
+ let unparser = Unparser::new(&dialect);
+ let sql = unparser.plan_to_sql(&plan)?.to_string();
+
+ assert_issue_23317_unparsed_sql_plans(&ctx, &sql).await?;
+
+ assert!(
+ sql.contains(r#"OVER (ORDER BY count(DISTINCT "customer_id")"#),
+ "window ORDER BY aggregate should resolve against the derived
projection output: {sql}",
+ );
+ assert!(
+ !sql.contains(r#"count(DISTINCT "cs"."customer_id")"#),
+ "window OVER clause must not reference out-of-scope alias cs: {sql}",
+ );
+
+ Ok(())
+}
+
+#[tokio::test]
+async fn optimized_duckdb_unparse_order_by_unqualifies_agg_input() ->
Result<()> {
+ let ctx = issue_23317_context()?;
+ assert!(ctx.remove_optimizer_rule(SingleDistinctToGroupBy::new().name()));
+
+ let plan = ctx
+ .sql(ISSUE_23668_ORDER_BY_QUERY)
+ .await?
+ .into_optimized_plan()?;
+ let dialect = DuckDBDialect::new();
+ let unparser = Unparser::new(&dialect);
+ let sql = unparser.plan_to_sql(&plan)?.to_string();
+
+ assert_issue_23317_unparsed_sql_plans(&ctx, &sql).await?;
+
+ assert!(
+ sql.contains(r#"ORDER BY round(sum("total_revenue"), 2)"#),
+ "ORDER BY aggregate should resolve against the derived projection
output: {sql}",
+ );
+ assert!(
+ !sql.contains(r#"sum("cs"."total_revenue")"#),
+ "ORDER BY must not reference out-of-scope alias cs: {sql}",
+ );
+
+ Ok(())
+}
+
+#[tokio::test]
+async fn optimized_duckdb_unparse_top_level_sort_over_agg_stays_in_scope() ->
Result<()> {
Review Comment:
This test is useful because it exercises the top-level `Sort` path, but
since it orders by the selected alias `customers`, it doesn't actually
demonstrate that the new `prepare_sort_expr` logic strips an aggregate input
qualifier in that branch.
Would it be worth adding a direct top-level `Sort` regression whose sort
expression contains an aggregate over a qualified input column? Alternatively,
the test name or comment could be adjusted to make it clear that it's only
covering the top-level `Sort` path. The non-selected aggregate regression
already covers the reported bug.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
