zabetak commented on code in PR #5294:
URL: https://github.com/apache/hive/pull/5294#discussion_r1684143833
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/RowResolver.java:
##########
@@ -125,6 +125,22 @@ private boolean putInternal(String tabAlias, String
colAlias, ColumnInfo colInfo
return false;
}
+ /**
+ * Puts a resolver entry corresponding to a aggregate expression and its
+ * column alias which is used for alias recognition in the having clause.
+ * @param exprToColumnAlias
+ * The map containing the expression to alias mapping
+ * @return
+ * @throws SemanticException
+ */
+ public void putAggregateAlias(Map<ASTNode, String> exprToColumnAlias) throws
SemanticException {
Review Comment:
The name of the method is unnecessarily tight to aggregates and aliases
while the implementation seems much more generic than that. Given that we just
call `put` for every entry in the map maybe a better name would be `putAll` or
maybe `replaceAll` since we only perform the action if an entry is already
there.
Will this logic really add an entry in the resolver or it just changes the
alias for ColumnInfo that is registered previously? Depending on the answer
another potential name would be `replaceAliases`.
A well chosen name will better indicate what the method really does without
requiring checking the implementation.
##########
ql/src/test/queries/clientpositive/having.q:
##########
@@ -18,4 +19,7 @@ EXPLAIN SELECT key, max(value) FROM src GROUP BY key HAVING
max(value) > "val_25
SELECT key, max(value) FROM src GROUP BY key HAVING max(value) > "val_255";
EXPLAIN SELECT key, COUNT(value) FROM src GROUP BY key HAVING count(value) >=
4;
-SELECT key, COUNT(value) FROM src GROUP BY key HAVING count(value) >= 4;
\ No newline at end of file
+SELECT key, COUNT(value) FROM src GROUP BY key HAVING count(value) >= 4;
+
+EXPLAIN CBO SELECT count(value) as c, max(key) as m from src GROUP BY key
HAVING c > 3 and m > 0;
+SELECT count(value) as c, max(key) as m from src GROUP BY key HAVING c > 3 and
m > 0;
Review Comment:
Since we are talking about `max(key)` does the `m > 0` condition filter
anything? If not then maybe changing to `m > 400` would be a safer choice.
--
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]
