deniskuzZ commented on code in PR #3637:
URL: https://github.com/apache/hive/pull/3637#discussion_r1000557331
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorGroupByOperator.java:
##########
@@ -1216,38 +1224,67 @@ protected void initializeOp(Configuration hconf) throws
HiveException {
}
@VisibleForTesting
- VectorAggregateExpression instantiateExpression(VectorAggregationDesc
vecAggrDesc,
- Configuration hconf) throws HiveException {
- Class<? extends VectorAggregateExpression> vecAggrClass =
vecAggrDesc.getVecAggrClass();
+ VectorAggregateExpression instantiateExpression(VectorAggregationDesc
vecAggrDesc) throws HiveException {
+ final Class<? extends VectorAggregateExpression> vecAggrClass =
vecAggrDesc.getVecAggrClass();
+ final Constructor<? extends VectorAggregateExpression> ctor;
+
+ final List<ConstantVectorExpression> constants =
+ vecAggrDesc.getConstants() == null ? Collections.emptyList() :
vecAggrDesc.getConstants();
+
+ final Class<?>[] ctorParamClasses = new Class<?>[constants.size() + 1];
+ ctorParamClasses[0] = VectorAggregationDesc.class;
+
+ final List<Object> values = new ArrayList<>(constants.size() + 1);
+ values.add(vecAggrDesc);
+
+ for (int i = 0; i < constants.size(); ++i) {
+ ConstantVectorExpression constant = constants.get(i);
+ String typeName = constant.getOutputTypeInfo().getTypeName();
+ if (!PRIMITIVE_TYPE_NAME_TO_CLASS.containsKey(typeName)) {
+ throw new IllegalArgumentException(
+ "Non-primitive type detected as " + i + "-th argument for a call
to the vectorized aggregation class "
+ + vecAggrClass.getSimpleName() + ", only primitive types are
supported");
+ }
+ ctorParamClasses[i + 1] = PRIMITIVE_TYPE_NAME_TO_CLASS.get(typeName);
+
+ // this is needed to bring back to the right type the value, e.g.
int-family always gets back a long,
+ // but in this way the constructor parameters won't match anymore, so we
need to convert here
+ switch (typeName) {
Review Comment:
based on the above recommendation, could we switch to
PrimitiveObjectInspector.PrimitiveCategory instead on String?
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorGroupByOperator.java:
##########
@@ -1216,38 +1224,67 @@ protected void initializeOp(Configuration hconf) throws
HiveException {
}
@VisibleForTesting
- VectorAggregateExpression instantiateExpression(VectorAggregationDesc
vecAggrDesc,
- Configuration hconf) throws HiveException {
- Class<? extends VectorAggregateExpression> vecAggrClass =
vecAggrDesc.getVecAggrClass();
+ VectorAggregateExpression instantiateExpression(VectorAggregationDesc
vecAggrDesc) throws HiveException {
+ final Class<? extends VectorAggregateExpression> vecAggrClass =
vecAggrDesc.getVecAggrClass();
+ final Constructor<? extends VectorAggregateExpression> ctor;
+
+ final List<ConstantVectorExpression> constants =
+ vecAggrDesc.getConstants() == null ? Collections.emptyList() :
vecAggrDesc.getConstants();
+
+ final Class<?>[] ctorParamClasses = new Class<?>[constants.size() + 1];
+ ctorParamClasses[0] = VectorAggregationDesc.class;
+
+ final List<Object> values = new ArrayList<>(constants.size() + 1);
+ values.add(vecAggrDesc);
+
+ for (int i = 0; i < constants.size(); ++i) {
+ ConstantVectorExpression constant = constants.get(i);
+ String typeName = constant.getOutputTypeInfo().getTypeName();
+ if (!PRIMITIVE_TYPE_NAME_TO_CLASS.containsKey(typeName)) {
+ throw new IllegalArgumentException(
+ "Non-primitive type detected as " + i + "-th argument for a call
to the vectorized aggregation class "
+ + vecAggrClass.getSimpleName() + ", only primitive types are
supported");
+ }
+ ctorParamClasses[i + 1] = PRIMITIVE_TYPE_NAME_TO_CLASS.get(typeName);
+
+ // this is needed to bring back to the right type the value, e.g.
int-family always gets back a long,
+ // but in this way the constructor parameters won't match anymore, so we
need to convert here
+ switch (typeName) {
Review Comment:
based on the above recommendation, could we switch to
PrimitiveObjectInspector.PrimitiveCategory instead of a String?
--
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]
