imply-cheddar commented on code in PR #14542:
URL: https://github.com/apache/druid/pull/14542#discussion_r1263224288
##########
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchBuildAggregatorFactory.java:
##########
@@ -221,11 +226,21 @@ private HllSketchUpdater
formulateSketchUpdater(ColumnSelectorFactory columnSele
}
};
break;
+ case ARRAY:
+ final ExpressionType expressionType =
ExpressionType.fromColumnTypeStrict(capabilities);
+ updater = sketch -> {
+ final Object o = selector.getObject();
+ if (o != null) {
+ byte[] bytes = ExprEval.toBytes(expressionType, o);
+ sketch.get().update(bytes);
+ }
+ };
+ break;
Review Comment:
But didn't you say that there could be places even during ingestion that an
`ARRAY` might show up as a `java.lang.List`? If we are adding the flag, it
seems easier to reason about and think consistently if it applies equivalent
across arrays and lists and we make the question of what happens at ingest time
a question of how the person writes the ingestion spec.
I.e. said another way, I think it's easier to document the parameter as a
flag that applies consistently to array-like things. Than it is to document it
as a parameter that applies to objects which end up as `java.lang.List` but not
to objecst that end up as an `Object[]` and then subsequently explain why and
when we expect an `Object[]` instead of a `java.lang.List` and how someone can
affect which object they are getting.
--
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]
