HIVE-29696: Vectorization: ClassCastException in VectorPTFGroupBatches when buffering complex window function inputs#6579
Open
Aggarwal-Raghav wants to merge 3 commits into
Open
Conversation
Contributor
Author
|
CC @abstractdog, because of HIVE-24761 |
Member
|
@Aggarwal-Raghav , thanks for the fix! please check unused imports |
Contributor
Author
Sure, will do once first level review is done and address them along with review comments. |
deniskuzZ
reviewed
Jul 6, 2026
Comment on lines
397
to
+503
| @@ -476,39 +489,27 @@ private void initBufferedColumns() { | |||
| } | |||
|
|
|||
| private void initExpressionColumns() { | |||
| for (int i = 0; i < evaluators.length; i++) { | |||
| VectorPTFEvaluatorBase evaluator = evaluators[i]; | |||
| for (VectorPTFEvaluatorBase evaluator : evaluators) { | |||
| /* | |||
| * Non-streaming evaluators work on buffered batches, we need to adapt them. Before PTF | |||
| * bounded start vectorization (HIVE-24761), VectorExpression.outputColumnNum was closed and | |||
| * VectorExpression.inputColumnNum didn't even exist (even though the vast majority of | |||
| * VectorExpression subclasses use at least 1 input column). Since VectorPTFOperator and | |||
| * VectorPTFGroupBatches work on modified batches (by not storing all the columns, and having | |||
| * ordering columns first), the expressions planned in compile-time won't work with the | |||
| * original config (column layout). It would make sense to move this logic to compile time, | |||
| * because here in runtime, a very simple mapping (bufferedColumnMap) is used, so it might be | |||
| * used. However, vectorized expression compilation affects many layers of code (having | |||
| * VectorizationContext as the common scope), and moving the calculation of bufferedColumnMap | |||
| * and this override logic to compile-time would create much more complicated behavior there | |||
| * (probably involving hacking most of the time, or maybe a great re-design) just because of | |||
| * the optimized column layout of the PTF vectorization. | |||
| * Non-streaming window function evaluators (like SUM or LEAD) work on packed, buffered batches. | |||
| * VectorPTFOperator and VectorPTFGroupBatches significantly modify the batch structure to save memory | |||
| * (e.g. by dropping unused columns and moving ordering columns to the front). | |||
| * | |||
| * Because of this dense packing, the absolute column indices assigned by the compiler (e.g., Index 15) | |||
| * are no longer valid for the buffer (where it might now be Index 2). | |||
| * Therefore, we must dynamically map the evaluator's inputColumnNum to its new location in the packed array. | |||
| * | |||
| * NOTE: We ONLY patch the evaluator's input index here. We specifically DO NOT patch | |||
| * evaluator.inputVecExpr (the math expressions like A+B) because those are now evaluated eagerly | |||
Member
There was a problem hiding this comment.
any perf impact after the change?
Contributor
Author
There was a problem hiding this comment.
I'm not an expert on vectorization but from the understanding while debugging this:
There is no performance degradation because the PTF operator does not filter rows; every row evaluated eagerly would have been lazily evaluated anyway. Infact, it provides benefits,
- Instead of writing raw inputs to a memory/disk buffer and fetching them back later to do the math
- By doing the math eagerly on the original batch, the buffering logic can drop the intermediate child scratch column (example Index 14) columns before buffering even starts. This means our
BufferedVectorizedRowBatchtakes up less RAM and disk footprint!"
Please let me know If I'm missing something.
flowchart TD
subgraph Lazy ["😬 BEFORE (Lazy Evaluation) - High Memory & Evaluation Overhead"]
direction TB
L1["1. Raw Data Arrives"]
L2["2. Pack into Buffer\n(Writes Raw Inputs to Memory / Disk)"]
L3["3. Partition Finishes"]
L4["4. Fetch rows back from Buffer"]
L5["5. Evaluate Math (A+B+C)\n(Creates intermediate Index 14 in RAM)"]
L6["6. Run SUM()"]
L1 --> L2 --> L3 --> L4 --> L5 --> L6
end
subgraph Eager ["😀 AFTER (Eager Evaluation) - Max Memory Efficiency"]
direction TB
E1["1. Raw Data Arrives"]
E2["2. Evaluate Math IMMEDIATELY\n(Index 1 ➔ Index 14 ➔ Index 15)"]
E3["3. Pack Final Answer into Buffer\n(Drops Index 14 to save RAM! Writes Index 15)"]
E4["4. Partition Finishes"]
E5["5. Run SUM()\n(Reads final answer directly)"]
E1 --> E2 --> E3 --> E4 --> E5
end
classDef bad fill:#f8d7da,stroke:#dc3545,stroke-width:2px;
classDef good fill:#d4edda,stroke:#28a745,stroke-width:2px;
classDef mem_win fill:#cce5ff,stroke:#004085,stroke-width:2px;
class L4,L5 bad;
class E1,E2 good;
class E3 mem_win;
Member
There was a problem hiding this comment.
makes sence!
btw lazy evaluation was introduced in https://github.com/apache/hive/pull/2278/changes, so let me cc @abstractdog
Contributor
Author
There was a problem hiding this comment.
Yes, It would be beneficial to get further insights
deniskuzZ
reviewed
Jul 7, 2026
deniskuzZ
reviewed
Jul 7, 2026
Contributor
Author
|
Sharing MORE Context, I observed 1 more stacktrace similar to this in |
asf-ci-hive
added
tests pending
tests passed
and removed
tests passed
tests pending
labels
deniskuzZ
reviewed
Jul 8, 2026
…s when buffering complex window function inputs
Aggarwal-Raghav
force-pushed
the
HIVE-29696
branch
from
d1fd80a to
df7d5be
Compare
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
What changes were proposed in this pull request?
Do Vector PTF input expressions evaluation eagerly on the original batch rather than lazily on the dense buffer.
Why are the changes needed?
To fix ClassCastExceptions in window functions during vectorized execution. When
VectorPTFOperatorbuffers rows, it compacts the column array to save memory, which shuffles the column indices. The old logic attempted to dynamically patch these indices but failed to traverse nested child expressions (like A + B + C )Does this PR introduce any user-facing change?
NO
How was this patch tested?
Wrote 2 q files