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.

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

Prevent take_optimizable from discarding arbitrary plan node #13030

Conversation

findepi
Copy link
Member

@findepi findepi commented Oct 21, 2024

take_optimizable started from inspecting top level plan node (it should be final aggregation) and then descended trying to find matching partial aggregation. When doing so, it would ignore any single-source nodes it passes by. As a result, AggregateStatistics could change the plan in an undesired manner.

Extracted from #13029

@findepi findepi marked this pull request as draft October 21, 2024 11:00
@github-actions github-actions bot added the optimizer Optimizer rules label Oct 21, 2024
`take_optimizable` started from inspecting top level plan node (it
should be final aggregation) and then descended trying to find matching
partial aggregation. When doing so, it would ignore any single-source
nodes it passes by. As a result, `AggregateStatistics` could change the
plan in an undesired manner.
@findepi findepi force-pushed the findepi/prevent-take-optimizable-from-discarding-arbitrary-plan-node-11408b branch from d54d101 to 7f67ab7 Compare October 21, 2024 11:28
@findepi
Copy link
Member Author

findepi commented Oct 21, 2024

this is not correct in strict sense -- if we defined Logical Plan semantics by defining each plan node separately as a function of its inputs, but maybe no-one gets hurt in the near term, given that in typical logical plans the final and partial won't be very far apart.

closing for now, to be fixed later.

@findepi findepi closed this Oct 21, 2024
@findepi findepi deleted the findepi/prevent-take-optimizable-from-discarding-arbitrary-plan-node-11408b branch October 21, 2024 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optimizer Optimizer rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant