Expand wildcard to actual expressions in `prepare_select_exprs` #15090

jayzhan211 · 2025-03-08T09:23:53Z

Which issue does this PR close?

Closes #.
A step before Remove Wildcard from Expr #7765 and Move ExpandWildcardRule into Logical Plan construction #14634
Before Move ExpandWildcardRule into Logical Plan construction #14634, other query like create view should be fixed as well

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

jayzhan211 · 2025-03-08T09:27:42Z

datafusion/sql/tests/cases/plan_to_sql.rs

-        TestStatementWithDialect {
-            sql: "select * from (select * from j1 limit 10);",
-            expected:
-                "SELECT * FROM (SELECT * FROM `j1` LIMIT 10) AS `derived_limit`",


If we expand * to all the columns, should we convert back to * for unparser?

I'm not sure should we support this, I delete related test and consider not supported

I think it's hard to convert back to * after removing Expr::Wildcard. It's okay for me to show all the column names for unparsing. They are equivalent.

jayzhan211 · 2025-03-08T09:28:22Z

datafusion/sql/src/select.rs

@@ -635,7 +642,15 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
                    planner_context,
                    options,
                )?;
-                Ok(wildcard_with_options(planned_options))
+
+                let expanded =


There two parts are the real change, others are test adjustment

jayzhan211 · 2025-03-08T09:29:16Z

datafusion/sql/tests/sql_integration.rs

@@ -54,15 +54,6 @@ use sqlparser::dialect::{Dialect, GenericDialect, HiveDialect, MySqlDialect};
 mod cases;
 mod common;

-#[test]
-fn test_schema_support() {


I move all the test including select * (those tests fail because they are expanded) to slt files. Add explain statement for them

jayzhan211 · 2025-03-09T01:40:15Z

datafusion/sqllogictest/test_files/order.slt

-# expect this query to run successfully, not error
-query III
+# b is not selected in subquery
+query error DataFusion error: Schema error: No field named b\. Valid fields are t1\.c, t1\.a, t1\.a0\.
 select * from (select c, a, NULL::int as a0 from t order by a, c) t1
 union all
 select * from (select c, NULL::int as a, a0 from t order by a0, c) t2
 order by c, a, a0, b


This query is executable in main, but it is not in this PR because b is not in the subquery and I think this is reasonable

I agree with you. This SQL isn't valid. However, it's added by #12721, which shows that issue #12446 has been solved. I do not understand the details of #12446. Maybe @alamb knows why we expect it works?

I will check and get back to you

I agree the query is invalid (and postgres agrees). I made a PR to fix it here:

Minor: Fix invalid query in test #15131

goldmedal · 2025-03-10T15:46:30Z

datafusion/sql/tests/cases/plan_to_sql.rs

-        TestStatementWithDialect {
-            sql: "select * from (select * from j1 limit 10);",
-            expected:
-                "SELECT * FROM (SELECT * FROM `j1` LIMIT 10) AS `derived_limit`",


I think it's hard to convert back to * after removing Expr::Wildcard. It's okay for me to show all the column names for unparsing. They are equivalent.

goldmedal · 2025-03-10T15:51:47Z

datafusion/sql/tests/cases/plan_to_sql.rs

        },
-        TestStatementWithDialect {
-            sql: "SELECT * FROM (SELECT j1_id + 1 FROM j1) AS temp_j(id2)",
-            expected: r#"SELECT * FROM (SELECT (`j1`.`j1_id` + 1) AS `id2` FROM `j1`) AS `temp_j`"#,
-            parser_dialect: Box::new(GenericDialect {}),
-            unparser_dialect: Box::new(SqliteDialect {}),
-        },


I think we can't remove them. They aren't related to the wildcard expansion. It's better to change the expected result to match the expanded result instead.

SELECT `temp_j`.`id2` FROM (SELECT (`j1`.`j1_id` + 1) AS `id2` FROM `j1`) AS `temp_j

goldmedal · 2025-03-10T16:17:53Z

datafusion/sqllogictest/test_files/order.slt

-# expect this query to run successfully, not error
-query III
+# b is not selected in subquery
+query error DataFusion error: Schema error: No field named b\. Valid fields are t1\.c, t1\.a, t1\.a0\.
 select * from (select c, a, NULL::int as a0 from t order by a, c) t1
 union all
 select * from (select c, NULL::int as a, a0 from t order by a0, c) t2
 order by c, a, a0, b


I agree with you. This SQL isn't valid. However, it's added by #12721, which shows that issue #12446 has been solved. I do not understand the details of #12446. Maybe @alamb knows why we expect it works?

goldmedal · 2025-03-10T16:21:18Z

datafusion/sqllogictest/test_files/order.slt

@@ -1231,44 +1238,20 @@ physical_plan


 # Test: inputs into union with different orderings
-query TT
+query error DataFusion error: Schema error: No field named d\. Valid fields are t1\.b, t1\.c, t1\.a, t1\.a0\.


Same as above. I'm worried about the test related to #12446 🤔

alamb · 2025-03-10T18:58:30Z

datafusion/sqllogictest/test_files/joins.slt

+07)----DataSourceExec: partitions=1, partition_sizes=[0]
+
+query TT
+explain SELECT * FROM person a NATURAL JOIN lineitem b;


it is very nice to port these tests to sqllogictests

alamb · 2025-03-10T19:06:28Z

datafusion/sqllogictest/test_files/order.slt

-# expect this query to run successfully, not error
-query III
+# b is not selected in subquery
+query error DataFusion error: Schema error: No field named b\. Valid fields are t1\.c, t1\.a, t1\.a0\.
 select * from (select c, a, NULL::int as a0 from t order by a, c) t1
 union all
 select * from (select c, NULL::int as a, a0 from t order by a0, c) t2
 order by c, a, a0, b


I agree the query is invalid (and postgres agrees). I made a PR to fix it here:

Minor: Fix invalid query in test #15131

goldmedal

Thanks @jayzhan211. It looks good to me 👍

goldmedal · 2025-03-11T11:20:26Z

datafusion/sql/tests/cases/plan_to_sql.rs

            parser_dialect: Box::new(GenericDialect {}),
            unparser_dialect: Box::new(UnparserDefaultDialect {}),
        },
        TestStatementWithDialect {
            sql: "SELECT * FROM UNNEST([1,2,3])",
-            expected: r#"SELECT * FROM UNNEST([1, 2, 3])"#,
+            expected: r#"SELECT UNNEST(make_array(Int64(1),Int64(2),Int64(3))) FROM UNNEST([1, 2, 3])"#,


umm.. The result is weird because the column name isn't quoted and is uncommon, but it's not related to this PR. I think we should have another PR for it. 🤔

I think we can change the name display for unnest array to unnest([..])

jayzhan211 · 2025-03-11T23:49:52Z

Thanks @goldmedal @alamb

jayzhan211 added 4 commits March 8, 2025 16:33

first draft

364e3a4

fix tests

cba1dff

cleanup

d195a5a

assert

ff071e8

github-actions bot added sql SQL Planner optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt) labels Mar 8, 2025

clippy

59f7e98

jayzhan211 commented Mar 8, 2025

View reviewed changes

jayzhan211 marked this pull request as ready for review March 8, 2025 10:00

jayzhan211 commented Mar 9, 2025

View reviewed changes

goldmedal self-requested a review March 10, 2025 01:31

goldmedal reviewed Mar 10, 2025

View reviewed changes

alamb mentioned this pull request Mar 10, 2025

Minor: Fix invalid query in test #15131

Merged

alamb reviewed Mar 10, 2025

View reviewed changes

jayzhan211 marked this pull request as draft March 11, 2025 00:43

jayzhan211 added 3 commits March 11, 2025 09:25

fix test

271abd3

Merge branch 'main' of github.com:apache/datafusion into mv-wildcard

deec729

fix test

09d14df

jayzhan211 marked this pull request as ready for review March 11, 2025 01:57

jayzhan211 requested review from alamb and goldmedal March 11, 2025 03:08

goldmedal approved these changes Mar 11, 2025

View reviewed changes

jayzhan211 merged commit 359735a into apache:main Mar 11, 2025
24 checks passed

jayzhan211 deleted the mv-wildcard branch March 11, 2025 23:49

This was referenced Mar 12, 2025

Reuse alias if possible #14781

Draft

Generate the common SQL for the unparsing result of the unnest #15233

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expand wildcard to actual expressions in `prepare_select_exprs` #15090

Expand wildcard to actual expressions in `prepare_select_exprs` #15090

jayzhan211 commented Mar 8, 2025 •

edited by alamb

Loading

jayzhan211 Mar 8, 2025

goldmedal Mar 10, 2025

jayzhan211 Mar 8, 2025

jayzhan211 Mar 8, 2025 •

edited

Loading

jayzhan211 Mar 9, 2025 •

edited

Loading

goldmedal Mar 10, 2025

alamb Mar 10, 2025

alamb Mar 10, 2025

goldmedal Mar 10, 2025

goldmedal Mar 10, 2025

goldmedal Mar 10, 2025

goldmedal Mar 10, 2025

alamb Mar 10, 2025

alamb Mar 10, 2025

goldmedal left a comment

goldmedal Mar 11, 2025

jayzhan211 Mar 11, 2025

jayzhan211 commented Mar 11, 2025

Expand wildcard to actual expressions in prepare_select_exprs #15090

Expand wildcard to actual expressions in prepare_select_exprs #15090

Conversation

jayzhan211 commented Mar 8, 2025 • edited by alamb Loading

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayzhan211 Mar 8, 2025 • edited Loading

Choose a reason for hiding this comment

jayzhan211 Mar 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goldmedal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayzhan211 commented Mar 11, 2025

Expand wildcard to actual expressions in `prepare_select_exprs` #15090

Expand wildcard to actual expressions in `prepare_select_exprs` #15090

jayzhan211 commented Mar 8, 2025 •

edited by alamb

Loading

jayzhan211 Mar 8, 2025 •

edited

Loading

jayzhan211 Mar 9, 2025 •

edited

Loading