goldmedal commented on code in PR #15090:
URL: https://github.com/apache/datafusion/pull/15090#discussion_r1987576889


##########
datafusion/sql/tests/cases/plan_to_sql.rs:
##########
@@ -524,96 +517,6 @@ fn roundtrip_statement_with_dialect() -> Result<()> {
             parser_dialect: Box::new(GenericDialect {}),
             unparser_dialect: Box::new(SqliteDialect {}),
         },
-        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 {}),
-        },

Review Comment:
   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
   ```



##########
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\.

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



##########
datafusion/sqllogictest/test_files/order.slt:
##########
@@ -985,13 +985,20 @@ drop table ambiguity_test;
 statement ok
 create table t(a0 int, a int, b int, c int) as values (1, 2, 3, 4), (5, 6, 7, 
8);
 
-# 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

Review Comment:
   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?



##########
datafusion/sql/tests/cases/plan_to_sql.rs:
##########
@@ -323,13 +323,6 @@ fn roundtrip_statement_with_dialect() -> Result<()> {
             parser_dialect: Box::new(MySqlDialect {}),
             unparser_dialect: Box::new(UnparserMySqlDialect {}),
         },
-        TestStatementWithDialect {
-            sql: "select * from (select * from j1 limit 10);",
-            expected:
-                "SELECT * FROM (SELECT * FROM `j1` LIMIT 10) AS 
`derived_limit`",

Review Comment:
   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.



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