alamb commented on code in PR #23176:
URL: https://github.com/apache/datafusion/pull/23176#discussion_r3508890070
##########
datafusion/core/tests/sql/unparser.rs:
##########
@@ -343,6 +343,62 @@ async fn
optimized_duckdb_unparse_preserves_derived_table_scope() -> Result<()>
Ok(())
}
+// https://github.com/apache/datafusion/issues/23138
+//
+// CSE on `coalesce(discount_pct, 0)` factors a shared CAST into an extra inner
+// projection, so `SubqueryAlias: o` ends up over two stacked projections. When
+// the unparser renders that as nested derived tables it must qualify the
+// pass-through `order_id` with a name in scope at each level -- it must not
+// rebase it to the outer subquery alias `o`, which is not visible inside the
+// inner derived table.
+const ISSUE_23138_QUERY: &str = r#"
+SELECT * FROM
+(
+ SELECT order_id FROM "warehouse"."main"."order_items"
+) oi
+JOIN (
+ SELECT order_id, coalesce(discount_pct, 0) AS discount_pct_2
+ FROM "warehouse"."main"."orders"
+) o USING (order_id)
+"#;
+
+#[tokio::test]
+async fn optimized_duckdb_unparse_qualifies_nested_passthrough_column() ->
Result<()> {
+ let ctx = issue_22961_context()?;
+ let plan = ctx.sql(ISSUE_23138_QUERY).await?.into_optimized_plan()?;
+ let dialect = DuckDBDialect::new();
+ let unparser = Unparser::new(&dialect);
+ let sql = unparser.plan_to_sql(&plan)?.to_string();
+
+ // The intermediate derived table has no `o` in scope, so the pass-through
Review Comment:
I recommend we simply compare to the expected unparsed query -- would make
it easier to understand what was actually being done / what was different
While the curernt checks for substrings might seem less brittle I think it
will be very hard to understand if/how they should be updated in the future if
the code changes
##########
datafusion/core/tests/sql/unparser.rs:
##########
@@ -343,6 +343,62 @@ async fn
optimized_duckdb_unparse_preserves_derived_table_scope() -> Result<()>
Ok(())
}
+// https://github.com/apache/datafusion/issues/23138
+//
+// CSE on `coalesce(discount_pct, 0)` factors a shared CAST into an extra inner
+// projection, so `SubqueryAlias: o` ends up over two stacked projections. When
+// the unparser renders that as nested derived tables it must qualify the
+// pass-through `order_id` with a name in scope at each level -- it must not
+// rebase it to the outer subquery alias `o`, which is not visible inside the
+// inner derived table.
+const ISSUE_23138_QUERY: &str = r#"
+SELECT * FROM
+(
+ SELECT order_id FROM "warehouse"."main"."order_items"
+) oi
+JOIN (
+ SELECT order_id, coalesce(discount_pct, 0) AS discount_pct_2
+ FROM "warehouse"."main"."orders"
+) o USING (order_id)
+"#;
+
+#[tokio::test]
+async fn optimized_duckdb_unparse_qualifies_nested_passthrough_column() ->
Result<()> {
+ let ctx = issue_22961_context()?;
+ let plan = ctx.sql(ISSUE_23138_QUERY).await?.into_optimized_plan()?;
+ let dialect = DuckDBDialect::new();
+ let unparser = Unparser::new(&dialect);
+ let sql = unparser.plan_to_sql(&plan)?.to_string();
+
+ // The intermediate derived table has no `o` in scope, so the pass-through
Review Comment:
You can add extra asserts for important properties (like order_id should be
unqualified)
--
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]
