Aggarwal-Raghav commented on code in PR #6579:
URL: https://github.com/apache/hive/pull/6579#discussion_r3528704331
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ptf/VectorPTFOperator.java:
##########
@@ -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
Review Comment:
I'm not an expert on vectorization but from the understanding while
debugging this:
There is absolutely 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 `BufferedVectorizedRowBatch` takes up
less RAM and disk footprint!"
```
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
