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

First and Last Accumulators should update with state row excluding is_set flag #7567

Closed
viirya opened this issue Sep 15, 2023 · 0 comments · Fixed by #7565
Closed

First and Last Accumulators should update with state row excluding is_set flag #7567

viirya opened this issue Sep 15, 2023 · 0 comments · Fixed by #7565
Labels
bug Something isn't working

Comments

@viirya
Copy link
Member

viirya commented Sep 15, 2023

Describe the bug

First and Last Accumulators would update itself from first/last row during merging state batches (e.g., merge_batch). However, currently it takes the whole state row (which includes is_set flag) into update_with_new_row which in turn takes all columns except for first one into orderings (so existing is_set is put there) and adds is_set flag. This ends with double is_set flags if state is called on the accumulators which have merged state batches.

Normally this is not an issue because state is not called once aggregation enters the stage of merging state batches. But in #7400, where spilling happens to call state on such accumulators to get its states and spill into disk. This leads to a hacky fix there and we should fix these two accumulators accordingly to avoid the hacky way.

To Reproduce

No response

Expected behavior

No response

Additional context

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
1 participant