clintropolis commented on a change in pull request #9638:
URL: https://github.com/apache/druid/pull/9638#discussion_r476301803



##########
File path: 
processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndex.java
##########
@@ -1135,21 +1117,21 @@ public MetricDesc(int index, AggregatorFactory factory)
       this.index = index;
       this.name = factory.getName();
 
-      String typeInfo = factory.getTypeName();
-      if ("float".equalsIgnoreCase(typeInfo)) {
-        capabilities = 
ColumnCapabilitiesImpl.createSimpleNumericColumnCapabilities(ValueType.FLOAT);
-        this.type = typeInfo;
-      } else if ("long".equalsIgnoreCase(typeInfo)) {
-        capabilities = 
ColumnCapabilitiesImpl.createSimpleNumericColumnCapabilities(ValueType.LONG);
-        this.type = typeInfo;
-      } else if ("double".equalsIgnoreCase(typeInfo)) {
-        capabilities = 
ColumnCapabilitiesImpl.createSimpleNumericColumnCapabilities(ValueType.DOUBLE);
-        this.type = typeInfo;
+      ValueType valueType = factory.getType();
+
+      if (valueType.isPrimitive()) {

Review comment:
       Hmm, that is a good point, this should probably check `isNumeric`. 
However, should the `else` be an `else if 
(ValueType.COMPLEX.equals(valueType))` to make sure it is actually is? I'm not 
sure the previous behavior of treating `STRING` as a `COMPLEX` was quite 
correct, it doesn't seem like it. Instead it should maybe handle `STRING` (and 
arrays) separately, though I'm not quite sure which capabilities it should have 
or if it should be an error case.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to