clintropolis commented on code in PR #16511:
URL: https://github.com/apache/druid/pull/16511#discussion_r1634250727
##########
processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java:
##########
@@ -186,6 +188,7 @@ public static Pair<Grouper<RowBasedKey>,
Accumulator<AggregateResult, ResultRow>
@Nullable final ReferenceCountingResourceHolder<ByteBuffer>
combineBufferHolder,
final int concurrencyHint,
final LimitedTemporaryStorage temporaryStorage,
+ final ObjectMapper jsonMapper,
final ObjectMapper spillMapper,
Review Comment:
should jsonMapper and spillMapper be different? I know spillMapper is
usually smile, though isn't results from historicals also smile? regardless,
probably worth a comment somewhere of why we have different mappers
##########
processing/src/main/java/org/apache/druid/segment/column/ObjectStrategyComplexTypeStrategy.java:
##########
@@ -39,22 +39,25 @@ public class ObjectStrategyComplexTypeStrategy<T>
implements TypeStrategy<T>
private final TypeSignature<?> typeSignature;
@Nullable
private final Hash.Strategy<T> hashStrategy;
+ @Nullable
+ private final Class<?> complexDimensionType;
public ObjectStrategyComplexTypeStrategy(ObjectStrategy<T> objectStrategy,
TypeSignature<?> signature)
{
- this(objectStrategy, signature, null);
+ this(objectStrategy, signature, null, null);
}
public ObjectStrategyComplexTypeStrategy(
ObjectStrategy<T> objectStrategy,
TypeSignature<?> signature,
- @Nullable final Hash.Strategy<T> hashStrategy
+ @Nullable final Hash.Strategy<T> hashStrategy,
+ @Nullable final Class<?> complexDimensionType
Review Comment:
should this just use `getClazz` of `ObjectStrategy`?
##########
processing/src/main/java/org/apache/druid/segment/column/TypeStrategy.java:
##########
@@ -216,4 +216,9 @@ default boolean equals(T a, T b)
{
throw DruidException.defensive("Not implemented. Check groupable() first");
}
+
+ default Class<?> complexDimensionType()
Review Comment:
nit: maybe `getComplexClass` or the classic `getClazz` would be a better
name since it probably isn't a different type when used as a dimension (and we
have the separate `groupable` to determine if it can be used as a dimension or
not)
--
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]
