Copilot commented on code in PR #6861:
URL: https://github.com/apache/hive/pull/6861#discussion_r4269373439
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java:
##########
@@ -3011,6 +3012,13 @@ private boolean validatePTFOperator(PTFOperator op,
VectorizationContext vContex
throw new RuntimeException("Unexpected window type " +
windowFrameDef.getWindowType());
}
+ // Row mode also accepts a non-constant ntile argument (read from the
first row of the partition).
+ if (supportedFunctionType == SupportedFunctionType.NTILE &&
+ !isPositiveIntConstant(singleExprNodeDesc)) {
+ setOperatorIssue(functionName + " only a positive integer constant
number of buckets is supported");
+ return false;
Review Comment:
`isPositiveIntConstant` only accepts constants whose runtime value is an
`Integer`. In Hive expression trees, numeric constants can legitimately be
represented as other numeric types (e.g., `Long`, `Short`, `Byte`, or writable
equivalents depending on constant folding / type inference), and those would be
incorrectly rejected and force row-mode even though the evaluator can handle
any positive value within int range. Consider accepting any `Number` (or using
the constant's `TypeInfo` + numeric conversion) and validating `> 0` and `<=
Integer.MAX_VALUE`.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java:
##########
@@ -5042,6 +5050,14 @@ private static ExprNodeDesc[]
getOrderExprNodeDescs(List<OrderExpressionDef> ord
return exprNodeDescs;
}
+ private static boolean isPositiveIntConstant(ExprNodeDesc exprNodeDesc) {
+ if (!(exprNodeDesc instanceof ExprNodeConstantDesc)) {
+ return false;
+ }
+ Object value = ((ExprNodeConstantDesc) exprNodeDesc).getValue();
+ return value instanceof Integer && (Integer) value > 0;
Review Comment:
`isPositiveIntConstant` only accepts constants whose runtime value is an
`Integer`. In Hive expression trees, numeric constants can legitimately be
represented as other numeric types (e.g., `Long`, `Short`, `Byte`, or writable
equivalents depending on constant folding / type inference), and those would be
incorrectly rejected and force row-mode even though the evaluator can handle
any positive value within int range. Consider accepting any `Number` (or using
the constant's `TypeInfo` + numeric conversion) and validating `> 0` and `<=
Integer.MAX_VALUE`.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java:
##########
@@ -3011,6 +3012,13 @@ private boolean validatePTFOperator(PTFOperator op,
VectorizationContext vContex
throw new RuntimeException("Unexpected window type " +
windowFrameDef.getWindowType());
}
+ // Row mode also accepts a non-constant ntile argument (read from the
first row of the partition).
+ if (supportedFunctionType == SupportedFunctionType.NTILE &&
+ !isPositiveIntConstant(singleExprNodeDesc)) {
+ setOperatorIssue(functionName + " only a positive integer constant
number of buckets is supported");
Review Comment:
This new validation message is grammatically awkward and a bit hard to
parse. Consider rephrasing to something like: `functionName + \" only supports
a positive integer constant number of buckets\"` or `functionName + \": number
of buckets must be a positive integer constant\"` to make EXPLAIN output
clearer.
--
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]
