clintropolis commented on code in PR #16577:
URL: https://github.com/apache/druid/pull/16577#discussion_r1631787651
##########
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)?
oops yes.
>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.
Ah yea that is reasonable i suppose, i think originally more were preserved
than not so i went with the copy. I guess `hasBitmapIndexes` isn't widely used
on query side anymore since stuff can just ask `ColumnIndexSupplier` for an
index and it will return if it has it, though i suppose it should still be
preserved just in 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.
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]
