gianm commented on code in PR #16577:
URL: https://github.com/apache/druid/pull/16577#discussion_r1631786003
##########
processing/src/main/java/org/apache/druid/segment/serde/NestedCommonFormatColumnPartSerde.java:
##########
@@ -303,10 +303,6 @@ public void read(ByteBuffer buffer, ColumnBuilder builder,
ColumnConfig columnCo
bitmapSerdeFactory,
byteOrder
);
- ColumnCapabilitiesImpl capabilitiesBuilder =
builder.getCapabilitiesBuilder();
Review Comment:
this is just removing unused code?
##########
processing/src/main/java/org/apache/druid/segment/virtual/ExpressionPlan.java:
##########
@@ -277,6 +277,9 @@ public ColumnCapabilities inferColumnCapabilities(@Nullable
ColumnType outputTyp
// until query time
return ColumnCapabilitiesImpl.copyOf(underlyingCapabilities)
.setType(ColumnType.STRING)
+ // this is sad, but currently
dictionary encodedness is tied to string
+ // selectors and sad stuff happens if
the input type isn't string
+
.setDictionaryEncoded(underlyingCapabilities.is(ValueType.STRING))
Review Comment:
shouldn't this be `underlyingCapabilities.isTrue() &&
underlyingCapabilities.is(ValueType.STRING)`?
##########
processing/src/main/java/org/apache/druid/segment/virtual/ExpressionPlan.java:
##########
@@ -277,6 +277,9 @@ public ColumnCapabilities inferColumnCapabilities(@Nullable
ColumnType outputTyp
// until query time
return ColumnCapabilitiesImpl.copyOf(underlyingCapabilities)
.setType(ColumnType.STRING)
+ // this is sad, but currently
dictionary encodedness is tied to string
+ // selectors and sad stuff happens if
the input type isn't string
+
.setDictionaryEncoded(underlyingCapabilities.is(ValueType.STRING))
Review Comment:
also, imo `copyOf` is not the right thing to use here, logically speaking. I
don't think we actually want to carry through all caps from the underlying
selector, just a few specific ones.
--
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]
