abstractdog commented on a change in pull request #1280:
URL: https://github.com/apache/hive/pull/1280#discussion_r469099535
##########
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:
1. constructor: first I tried to pass it to constructor, but that breaks
compatibility with every other subclasses of VectorAggregateExpression, if I
use ctor.newInstance(vecAggrDesc, hconf), I need to add that constructor to all
subclasses, because they don't inherit this ctor from
VectorAggregateExpression...withConf can solve this, let me know about better
ways
2. single int: this config is specific to VectorUDAFBloomFilterMerge, so I
believe I should not pass it through a constructor to every
VectorAggregateExpression, and I didn't want to go for an instanceof hack for a
cast + specific call
----------------------------------------------------------------
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]
