abstractdog commented on a change in pull request #1280:
URL: https://github.com/apache/hive/pull/1280#discussion_r470419947
##########
File path:
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorGroupByOperator.java
##########
@@ -1126,6 +1137,7 @@ protected void initializeOp(Configuration hconf) throws
HiveException {
VectorAggregateExpression vecAggrExpr = null;
try {
vecAggrExpr = ctor.newInstance(vecAggrDesc);
+ vecAggrExpr.withConf(hconf);
Review comment:
Sadly, I need to agree with conf abusing in (hive) codebase :) somehow I
don't really like instanceof stuff here, only for a single expression,
moreover, I wanted to find a general way to provide some configuration to
expressions, as this patch showed that they might need that (in the future). On
the other hand, explicitly calling a specific constructor for different types
could be a kind of documentation in one place about "how to instantiate" these
expressions. I'm about to refactor this logic to a separate method in
VectorGroupByOperator and let this patch go!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
