kosiew commented on code in PR #23176:
URL: https://github.com/apache/datafusion/pull/23176#discussion_r3490324247


##########
datafusion/core/tests/sql/unparser.rs:
##########
@@ -343,6 +343,43 @@ 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
+    // `order_id` must be unqualified there, not rebased to the subquery alias
+    // `o` (which is only the base-table alias one level deeper). The bug 
emitted
+    // `"o"."order_id"` inside that derived table; the fix emits a bare column.
+    assert!(sql.contains(r#"(SELECT "order_id", CASE WHEN"#));

Review Comment:
   Nice regression test. One thought: would it make sense to make the positive 
assertion a little less tied to the exact emitted `CASE WHEN` formatting? For 
example, you could isolate the intermediate derived table fragment and check 
that `order_id` is unqualified there. That would still cover the alias scope 
bug while making the test a bit more resilient to unrelated formatting changes.



-- 
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]

Reply via email to