gianm commented on code in PR #11201:
URL: https://github.com/apache/druid/pull/11201#discussion_r1247262497
##########
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchMergeAggregatorFactory.java:
##########
@@ -41,6 +42,12 @@
/**
* This aggregator factory is for merging existing sketches.
* The input column must contain {@link HllSketch}
+ *
+ * Note: aggregators generated by this class do not directly use
"stringEncoding", but it is part of this class
+ * anyway so we can preserve enough information to ensure that we are merging
sketches in a valid way. (Sketches with
+ * incompatible string encodings cannot be merged meaningfully.) Currently,
the only way this is exposed is through
+ * {@link #getMergingFactory}, which will throw {@link
AggregatorFactoryNotMergeableException} if presented with
+ * two aggregators with two different encodings.
Review Comment:
Yes, that's true, there's no real way to discover this. That's a big part of
why the functionality is undocumented in this patch. I think we need to think a
bit through how to expose it in a way that is safe for users. However, I do
want to get it in, because having the functionality available will allow us to
do further experimentation.
--
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]
