abhishekagarwal87 commented on code in PR #15095:
URL: https://github.com/apache/druid/pull/15095#discussion_r1354546464
##########
sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java:
##########
@@ -313,12 +328,48 @@ public RelDataType inferReturnType(SqlOperatorBinding
sqlOperatorBinding)
}
}
+ private static class TimeColIdentifer extends SqlIdentifier
+ {
+
+ public TimeColIdentifer()
+ {
+ super("__time", SqlParserPos.ZERO);
+ }
+
+ @Override
+ public <R> R accept(SqlVisitor<R> visitor)
+ {
+
+ try {
+ return super.accept(visitor);
+ }
+ catch (CalciteContextException e) {
+ if (e.getCause() instanceof SqlValidatorException) {
+ throw DruidException.forPersona(DruidException.Persona.ADMIN)
+
.ofCategory(DruidException.Category.INVALID_INPUT)
+ .build(
+ e,
+ "Query could not be planned. A possible
reason is [%s]",
Review Comment:
please add a test case to validate the error message.
##########
sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java:
##########
@@ -313,12 +328,48 @@ public RelDataType inferReturnType(SqlOperatorBinding
sqlOperatorBinding)
}
}
+ private static class TimeColIdentifer extends SqlIdentifier
+ {
+
+ public TimeColIdentifer()
+ {
+ super("__time", SqlParserPos.ZERO);
+ }
+
+ @Override
+ public <R> R accept(SqlVisitor<R> visitor)
+ {
+
+ try {
+ return super.accept(visitor);
+ }
+ catch (CalciteContextException e) {
+ if (e.getCause() instanceof SqlValidatorException) {
+ throw DruidException.forPersona(DruidException.Persona.ADMIN)
Review Comment:
```suggestion
throw DruidException.forPersona(DruidException.Persona.USER)
```
##########
sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java:
##########
@@ -339,6 +390,43 @@ private static class EarliestLatestSqlAggFunction extends
SqlAggFunction
false,
Optionality.FORBIDDEN
);
+ this.replacementAggFunc = replacementAggFunc;
+ }
+
+ @Override
+ public SqlNode rewriteCall(
+ SqlValidator validator,
+ SqlCall call
+ )
+ {
+ // Rewrite EARLIEST/LATEST to EARLIEST_BY/LATEST_BY to make
+ // reference to __time column explicit so that Calcite tracks it
+
+ if (replacementAggFunc == null) {
+ return call;
+ }
+
+ List<SqlNode> operands = call.getOperandList();
+
+ SqlParserPos pos = call.getParserPosition();
+
+ if (operands.isEmpty() || operands.size() > 2) {
+ throw InvalidSqlInput.exception(
+ "Function [%s] expects 1 or 2 arguments but found [%s]",
+ getName(),
+ operands.size()
+ );
+ }
+
+ List<SqlNode> newOperands = new ArrayList<>();
+ newOperands.add(operands.get(0));
+ newOperands.add(new TimeColIdentifer());
+
+ if (operands.size() == 2) {
+ newOperands.add(operands.get(1));
+ }
Review Comment:
would we ever get here?
--
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]
