clintropolis commented on code in PR #14195:
URL: https://github.com/apache/druid/pull/14195#discussion_r1295027866
##########
sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java:
##########
@@ -89,7 +91,7 @@ AggregatorFactory createAggregatorFactory(String name, String
fieldName, String
return new DoubleFirstAggregatorFactory(name, fieldName,
timeColumn);
case STRING:
case COMPLEX:
- return new StringFirstAggregatorFactory(name, fieldName,
timeColumn, maxStringBytes);
+ return new StringFirstAggregatorFactory(name, fieldName,
timeColumn, null, maxStringBytes, true);
Review Comment:
is this right if we go forward with separate `foldColumn`? Shouldn't the
complex case put `fieldName` in the `foldColumn`?
If you drop `foldColumn` and use the selector factory + capabilities (my
preference at least at the time of writing this comment) instead then this
doesn't need to change for now I think. Though, this part will also need to
change if the PR that adds ingest time support for numeric first/last, since
we'll need to get the complex type name and pick the correct folding
aggregator...
--
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]
