Skip to content

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