abhishekrb19 commented on code in PR #14352:
URL: https://github.com/apache/druid/pull/14352#discussion_r1212469180
##########
sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestBySqlAggregator.java:
##########
@@ -168,12 +168,12 @@ private static class EarliestByLatestBySqlAggFunction
extends SqlAggFunction
InferTypes.RETURN_TYPE,
OperandTypes.or(
OperandTypes.sequence(
- "'" + aggregatorType.name() + "(expr, timeColumn)'\n",
+ "'" + StringUtils.format("%s_BY", aggregatorType.name()) +
"(expr, timeColumn)'\n",
Review Comment:
@clintropolis, yes, the single vs no quote inconsistency is everywhere in
this area. I'm not sure there's a "right" way to do this since there seems to
be exceptions inside the Calcite layer itself (`GROUPING`, for example). For
the most part when `OperandTypes.sequence` is not present, Calcite gives back a
string with single quotes in them, but again there are exceptions for things
defined in Calcite internally 😅 . I guess one option is to sanitize the strings
by trimming the single quotes (or include them), but I don't feel too strongly
about it. We could also do it in the web-console if needed. What do you think?
--
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]
