findepi commented on code in PR #17520:
URL: https://github.com/apache/datafusion/pull/17520#discussion_r2341308087
##########
datafusion/sqllogictest/test_files/projection.slt:
##########
@@ -252,3 +252,30 @@ physical_plan
statement ok
drop table t;
+
+# Regression test for 17513
+
+query I
+COPY (select 1 as a, 2 as b)
+TO 'test_files/scratch/projection/17513.parquet'
+STORED AS PARQUET;
+----
+1
+
+statement ok
+create external table t1 stored as parquet location
'test_files/scratch/projection/17513.parquet';
+
+query TT
+explain format indent
+select from t1 where t1.a > 1;
Review Comment:
Plan tests are cool, but plan changes are easy to overlook. They need manual
verification.
Query results-based tests are less disputable.
Can we also run this query and assert the result?
sqllogic doesn't support this (maybe we would need to change the rendering
or result lines to make them non-empty).
If slt doesn't allow testing this, then we need a test in Rust.
##########
datafusion/sql/src/select.rs:
##########
@@ -676,13 +676,12 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
let mut prepared_select_exprs = vec![];
let mut error_builder = DataFusionErrorBuilder::new();
- // Handle the case where no projection is specified but we have a
valid FROM clause
- // In this case, implicitly add a wildcard projection (SELECT *)
- let projection = if projection.is_empty() && !empty_from {
- vec![SelectItem::Wildcard(WildcardAdditionalOptions::default())]
- } else {
- projection
- };
+ // Note: We previously added wildcard projection for empty projections
to support
+ // FROM-first syntax, but this caused a regression where "SELECT FROM
table"
+ // incorrectly returned all columns instead of empty projection.
+ // For now, we don't add wildcard, which fixes the regression but
breaks FROM-first.
+ // A proper fix would need to distinguish between "FROM table" and
"SELECT FROM table"
+ // at the parser level, which sqlparser currently doesn't support.
Review Comment:
I am not sure the comment is needed. The test is an automated comment.
This PR is logically a revert + regression test.
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
