fix: Fix optimization of re-using inputVector #12332
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: D69567930 introduced an optimization to reuse the inputVector to save cycles and likely memory when extracting data from a group. However, there is a subtle bug here when we call `extractSingleGroup()`, it can return 0 rows. In this case, we must still update the firstInputColumn, which is a rolling index that represents which inputVectors map to which aggregateInputs. By not updating this, we may get a mismatch in the reused inputVector and the actual underlying type at this particular column. This is a hidden bug that surfaces if the following conditions are at least satisfied: 1. We've extracted a group with 0 rows 2. We've retried to use an index at a position which has a type that is different that the column index. Differential Revision: D69625562
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D69625562 |
✅ Deploy Preview for meta-velox canceled.
|
Yuhta
approved these changes
Feb 14, 2025
|
This pull request has been merged in 1f12637. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
D69567930 introduced an optimization to reuse the inputVector to save cycles and likely memory when extracting data from a group.
However, there is a subtle bug here when we call
extractSingleGroup(), it can return 0 rows. In this case, we must still update the firstInputColumn, which is a rolling index that represents which inputVectors map to which aggregateInputs.By not updating this, we may get a mismatch in the reused inputVector and the actual underlying type at this particular column. This is a hidden bug that surfaces if the following conditions are at least satisfied:
Differential Revision: D69625562