LakshSingla commented on code in PR #15670:
URL: https://github.com/apache/druid/pull/15670#discussion_r1452961591
##########
sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestBySqlAggregator.java:
##########
@@ -165,6 +174,29 @@ public Aggregation toDruidAggregation(
);
}
+ private static boolean isMetricPreAggregated(PlannerContext plannerContext,
RexNode rexNode, String byAggregator)
+ {
+ final RelDataType type = rexNode.getType();
+ if (type instanceof RowSignatures.ComplexSqlType) {
+ String complexColumnTypeName = ((RowSignatures.ComplexSqlType)
type).getColumnType().getComplexTypeName();
+ if
((SerializablePairLongLongComplexMetricSerde.TYPE_NAME.equals(complexColumnTypeName)
+ ||
SerializablePairLongFloatComplexMetricSerde.TYPE_NAME.equals(complexColumnTypeName)
+ ||
SerializablePairLongDoubleComplexMetricSerde.TYPE_NAME.equals(complexColumnTypeName)
+ ||
SerializablePairLongStringComplexMetricSerde.TYPE_NAME.equals(complexColumnTypeName)))
{
+ plannerContext.setPlanningError(
+ "Cannot call %s with an explicit 'timeExpr' column for
pre-aggregated metric of type [%s]. Use %s instead "
Review Comment:
Thanks for the comment. I tried to add an `isReplaced` flag to distinguish
between the two calls, but it seems like during the SQL planning phase of
calcite, it rewrites the call to lose that information and uses the same SQL
function for both variants (earliest, earliest_by). I cannot do a type check on
the custom __time operand that was passed, therefore the next best resort is to
just confirm if the identifier is __time or not.
This would mean `EARLIEST_BY(complexMetric, __time)` would also pass, but I
couldn't come up with an easy fix for the same.
Since these will be deprecated sometime, I think its an acceptable
compromise.
--
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]
