deniskuzZ commented on code in PR #6579:
URL: https://github.com/apache/hive/pull/6579#discussion_r3528424969
##########
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:
any perf impact after the change?
--
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]
