leventov commented on a change in pull request #6220: Fix four bugs with
numeric dimension output types.
URL: https://github.com/apache/incubator-druid/pull/6220#discussion_r212519103
##########
File path:
processing/src/main/java/io/druid/query/topn/types/TopNColumnSelectorStrategyFactory.java
##########
@@ -27,23 +28,39 @@
public class TopNColumnSelectorStrategyFactory implements
ColumnSelectorStrategyFactory<TopNColumnSelectorStrategy>
{
+ private final ValueType dimensionType;
+
+ public TopNColumnSelectorStrategyFactory(final ValueType dimensionType)
+ {
+ this.dimensionType = Preconditions.checkNotNull(dimensionType,
"dimensionType");
+ }
+
@Override
public TopNColumnSelectorStrategy makeColumnSelectorStrategy(
ColumnCapabilities capabilities, ColumnValueSelector selector
)
{
- ValueType type = capabilities.getType();
- switch (type) {
+ final ValueType selectorType = capabilities.getType();
+
+ switch (selectorType) {
case STRING:
- return new StringTopNColumnSelectorStrategy();
+ // Return strategy that reads strings and outputs dimensionTypes.
+ return new StringTopNColumnSelectorStrategy(dimensionType);
case LONG:
- return new NumericTopNColumnSelectorStrategy.OfLong();
case FLOAT:
- return new NumericTopNColumnSelectorStrategy.OfFloat();
case DOUBLE:
- return new NumericTopNColumnSelectorStrategy.OfDouble();
+ if (ValueType.isNumeric(dimensionType)) {
+ // Return strategy that aggregates using the _output_ type, because
this allows us to collapse values
+ // properly (numeric types cannot represent all values of other
numeric types).
+ return NumericTopNColumnSelectorStrategy.ofType(dimensionType,
dimensionType);
+ } else {
+ // Return strategy that aggregates using the _input_ type. Here we
are assuming that the output type can
Review comment:
Despite those comments, it's still hard to understand what is going on here.
Maybe you could try to clarify it more?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
