soumyakanti3578 commented on code in PR #6212:
URL: https://github.com/apache/hive/pull/6212#discussion_r2579094372
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java:
##########
@@ -1000,7 +1001,18 @@ public ASTNode visitOver(RexOver over) {
}
// 1. Translate the UDAF
- final ASTNode wUDAFAst = visitCall(over);
+ ASTNode wUDAFAst = ASTBuilder.createAST(HiveParser.TOK_FUNCTION,
"TOK_FUNCTION");
Review Comment:
Maybe add a few lines of comments to explain what it is doing, but not
strictly required.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java:
##########
@@ -1000,7 +1001,18 @@ public ASTNode visitOver(RexOver over) {
}
// 1. Translate the UDAF
- final ASTNode wUDAFAst = visitCall(over);
+ ASTNode wUDAFAst = ASTBuilder.createAST(HiveParser.TOK_FUNCTION,
"TOK_FUNCTION");
+ if (over.getOperands().isEmpty() && over.op.getSyntax() ==
SqlSyntax.FUNCTION_STAR) {
+ wUDAFAst = ASTBuilder.createAST(HiveParser.TOK_FUNCTIONSTAR,
"TOK_FUNCTIONSTAR");
+ }
+ if (over.isDistinct()) {
+ wUDAFAst = ASTBuilder.createAST(HiveParser.TOK_FUNCTIONDI,
"TOK_FUNCTIONDI");
+ }
+ wUDAFAst.addChild(ASTBuilder.createAST(HiveParser.Identifier,
over.op.getName()));
+ wUDAFAst.setTypeInfo(TypeConverter.convert(over.type));
+ for (RexNode operand : over.getOperands()) {
+ wUDAFAst.addChild(operand.accept(this));
+ }
Review Comment:
Adding a few lines of comments explaining this part would be nice as it has
changed significantly.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/jdbc/JDBCAggregationPushDownRule.java:
##########
@@ -62,8 +62,7 @@ public boolean matches(RelOptRuleCall call) {
SqlAggFunction f = relOptRuleOperand.getAggregation();
if (f instanceof HiveSqlCountAggFunction) {
//count distinct with more that one argument is not supported
- HiveSqlCountAggFunction countAgg = (HiveSqlCountAggFunction)f;
- if (countAgg.isDistinct() && 1 <
relOptRuleOperand.getArgList().size()) {
+ if (relOptRuleOperand.isDistinct() && 1 <
relOptRuleOperand.getArgList().size()) {
Review Comment:
nit: If you are changing this line, can we flip the second predicate so that
the constant is on the right side? Makes this more readable in my opinion.
--
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]
