Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Optimizer adds redundant projections on casts with confusing alias #3786

Open
andygrove opened this issue Oct 10, 2022 · 8 comments
Open

Optimizer adds redundant projections on casts with confusing alias #3786

andygrove opened this issue Oct 10, 2022 · 8 comments
Labels
bug Something isn't working optimizer Optimizer rules

Comments

@andygrove
Copy link
Member

Describe the bug

#3764 fixed one regression but introduced another.

I do not understand why this additional projection is being added.

Before

Inner Join: store_sales.ss_sold_date_sk = date_dim.d_date_sk

After

Projection: CAST(date_dim.d_moy AS Int64) AS CAST(date_dim.d_moy AS Int64)date_dim.d_moy, store_sales.ss_customer_sk, date_dim.d_year
  Inner Join: store_sales.ss_sold_date_sk = date_dim.d_date_sk

To Reproduce
No repro available yet

Expected behavior

Additional context
@andygrove andygrove added bug Something isn't working optimizer Optimizer rules labels Oct 10, 2022
@andygrove andygrove changed the title Optimizer reduntant projections on casts with confusing alias Optimizer adds reduntant projections on casts with confusing alias Oct 10, 2022
@andygrove andygrove changed the title Optimizer adds reduntant projections on casts with confusing alias Optimizer adds redundant projections on casts with confusing alias Oct 10, 2022
@alamb
Copy link
Contributor

alamb commented Oct 11, 2022

Note I tried to explain in #3794 why aliases are needed for optimizer rewrite passes in general.

This may also be related to #3770

@alamb
Copy link
Contributor

alamb commented Oct 11, 2022

Using EXPLAIN VERBOSE on the plan should reveal which optimizer pass is making this change. If I had to guess, I would guess CommonSubexprEliminate

@liukun4515
Copy link
Contributor

Maybe related to this issue #3635 which will produce redundant projection after run the optimizer

@andygrove
Copy link
Member Author

Another example:

Projection: household_demographics.hd_vehicle_count > Int32(0) AS household_demographics.hd_vehicle_count > Int32(0)Int32(0)household_demographics.hd_vehicle_count

The alias name should just be:

household_demographics.hd_vehicle_count > Int32(0)

and not

household_demographics.hd_vehicle_count > Int32(0)Int32(0)household_demographics.hd_vehicle_count

@andygrove andygrove mentioned this issue Oct 25, 2022
Merged
@andygrove
Copy link
Member Author

Another example in #3884 (comment)

@andygrove andygrove self-assigned this Oct 25, 2022
@andygrove andygrove added this to the 14.0.0 milestone Oct 30, 2022
@andygrove
Copy link
Member Author

andygrove commented Nov 1, 2022 •
edited
Loading

I confirmed that this does happen in common_sub_expression_eliminate

@andygrove andygrove mentioned this issue Nov 1, 2022
Closed
@andygrove andygrove removed their assignment Nov 1, 2022
@andygrove
Copy link
Member Author

Fixing this will involve a significant refactor/rewrite of portions of common_sub_expression_eliminate

@andygrove andygrove removed this from the 14.0.0 milestone Nov 1, 2022
@andygrove andygrove mentioned this issue Nov 16, 2022
Closed
@jackwener
Copy link
Member

related with #4575

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working optimizer Optimizer rules
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

remove confusing aliases from aggregate expressions andygrove/datafusion
4 participants
@alamb @andygrove @liukun4515 @jackwener