clintropolis commented on a change in pull request #12130:
URL: https://github.com/apache/druid/pull/12130#discussion_r781899862
##########
File path:
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchBuildAggregator.java
##########
@@ -55,7 +69,10 @@ public ArrayOfDoublesSketchBuildAggregator(
this.valueSelectors = valueSelectors.toArray(new
BaseDoubleColumnValueSelector[0]);
values = new double[valueSelectors.size()];
sketch = new
ArrayOfDoublesUpdatableSketchBuilder().setNominalEntries(nominalEntries)
- .setNumberOfValues(valueSelectors.size()).build();
+
.setNumberOfValues(valueSelectors.size()).build();
+
+ this.canCacheById = this.keySelector.nameLookupPossibleInAdvance();
Review comment:
I think it might be more correct to use
`ColumnCapabilities.areDictionaryValuesUnique` instead of
`DimensionDictionarySelector.nameLookupPossibleInAdvance`, because the latter
does not necessarily require that dictionary ids uniquely identify lookup
values, just that it can be done without the cursor being on the current row
that provided the dictionary ids. `IndexedTable` selectors for example use the
row number as the dictionary id iirc. The capabilities could be retrieved from
the column selector factory available in the factorize methods to pass in I
think.
--
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]
