alamb commented on code in PR #4141:
URL: https://github.com/apache/arrow-datafusion/pull/4141#discussion_r1017146483
##########
datafusion/core/tests/sql/explain_analyze.rs:
##########
@@ -746,7 +746,7 @@ async fn test_physical_plan_display_indent_multi_children()
{
" ProjectionExec: expr=[c2@0 as c2]",
" ProjectionExec: expr=[c1@0 as c2]",
" RepartitionExec: partitioning=RoundRobinBatch(9000)",
- " CsvExec:
files=[ARROW_TEST_DATA/csv/aggregate_test_100.csv], has_header=true,
limit=None, projection=[c1]",
+ " CsvExec:
files=[ARROW_TEST_DATA/csv/aggregate_test_100.csv], has_header=true,
limit=None, projection=[c1, c2]",
Review Comment:
This doesn't seem right to me.
The query is only selecting `c1` from the table -- so why is this scan now
selecting c1 *and* c2?
```
let sql = "SELECT c1 \
FROM (select c1 from aggregate_test_100) AS a \
JOIN\
(select c1 as c2 from aggregate_test_100) AS b \
ON c1=c2\
";
```
##########
datafusion/core/tests/sql/joins.rs:
##########
@@ -1521,8 +1521,8 @@ async fn reduce_left_join_3() -> Result<()> {
"Explain [plan_type:Utf8, plan:Utf8]",
" Projection: t3.t1_id, t3.t1_name, t3.t1_int, t2.t2_id, t2.t2_name,
t2.t2_int [t1_id:UInt32;N, t1_name:Utf8;N, t1_int:UInt32;N, t2_id:UInt32;N,
t2_name:Utf8;N, t2_int:UInt32;N]",
" Left Join: t3.t1_int = t2.t2_int [t1_id:UInt32;N, t1_name:Utf8;N,
t1_int:UInt32;N, t2_id:UInt32;N, t2_name:Utf8;N, t2_int:UInt32;N]",
- " Projection: t3.t1_id, t3.t1_name, t3.t1_int, alias=t3
[t1_id:UInt32;N, t1_name:Utf8;N, t1_int:UInt32;N]",
- " Projection: t1.t1_id, t1.t1_name, t1.t1_int, alias=t3
[t1_id:UInt32;N, t1_name:Utf8;N, t1_int:UInt32;N]",
Review Comment:
it sure looks better to have not have this double alias
##########
benchmarks/expected-plans/q13.txt:
##########
@@ -2,8 +2,8 @@ Sort: custdist DESC NULLS FIRST, c_orders.c_count DESC NULLS
FIRST
Projection: c_orders.c_count, COUNT(UInt8(1)) AS custdist
Aggregate: groupBy=[[c_orders.c_count]], aggr=[[COUNT(UInt8(1))]]
Projection: c_orders.COUNT(orders.o_orderkey) AS c_count, alias=c_orders
- Projection: c_orders.COUNT(orders.o_orderkey), alias=c_orders
- Projection: COUNT(orders.o_orderkey), alias=c_orders
+ Projection: COUNT(orders.o_orderkey), alias=c_orders
+ Projection: COUNT(orders.o_orderkey)
Review Comment:
This redundant projection seems unfortunate (though not caused by this PR)
##########
datafusion/sql/src/planner.rs:
##########
@@ -3170,10 +3170,10 @@ mod tests {
) AS a
) AS b";
let expected = "Projection: b.fn2, b.last_name\
- \n Projection: b.fn2, b.last_name, b.birth_date,
alias=b\
- \n Projection: a.fn1 AS fn2, a.last_name,
a.birth_date, alias=b\
Review Comment:
yeah looking at these old plans now this selecting a just to alias them as
`b` is a little strange
--
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]
