Uh oh!
There was an error while loading. Please reload this page.
fix regression in the use of name in ProjectionPushdown - #8219
Conversation
alamb
commented
Nov 15, 2023
FYI @gruuya |
| let mut new_aggr_expr = vec![]; | ||
| for e in agg.aggr_expr.iter() { | ||
| let column = Column::from(e.display_name()?); | ||
| let column = Column::from_name(e.display_name()?); |
There was a problem hiding this comment.
This change was done in #7981 -- it doesn't seem necessary and I think was just a mistake (and did not have adequate test coverage)
There was a problem hiding this comment.
Thanks for resolving this; this was a leftover from my initial attempt to address the qualified alias problem (without altering the Expr::Alias enum) that I forgot to rollback.
There was a problem hiding this comment.
No problem. The real problem is that we had no test coverage for it :(
comphead
left a comment
There was a problem hiding this comment.
lgtm,fromfrom_namefrom_qualified_name
might be confusing, we may want to have better naming?
alamb
commented
Nov 15, 2023
It is very confusing and results in subtle bugs. I'll see if I can find some way to make it better as a follow on PR |
alamb
commented
Nov 15, 2023
Thank you for the quick review @comphead |
Which issue does this PR close?
Resolves#8223
Rationale for this change
We found a bug while upgrading IOx to use the latest datafusion
Some of our queries began failing like this:
The regression was introduced in #7981 and is related to columns with periods in their names
What changes are included in this PR?
DISTINCT ONfrom Postgres #7981Are these changes tested?
yes
Are there any user-facing changes?
bug fix