clintropolis commented on a change in pull request #10248:
URL: https://github.com/apache/druid/pull/10248#discussion_r468258415
##########
File path:
processing/src/main/java/org/apache/druid/segment/DimensionHandlerUtils.java
##########
@@ -73,16 +73,16 @@ private DimensionHandlerUtils()
)
{
if (capabilities == null) {
- return new StringDimensionHandler(dimensionName, multiValueHandling,
true);
+ return new StringDimensionHandler(dimensionName, multiValueHandling,
true, false);
}
multiValueHandling = multiValueHandling == null ?
MultiValueHandling.ofDefault() : multiValueHandling;
if (capabilities.getType() == ValueType.STRING) {
- if (!capabilities.isDictionaryEncoded()) {
+ if (!capabilities.isDictionaryEncoded().isMaybeTrue()) {
Review comment:
I hope this doesn't make this more confusing 😜
Unknowns are no longer expected to be here, but the unknowns which could
previously appear here, prior to the coercion logic refactoring, should have
been treated as true anyway due to the context of how this method is called.
This is because unknown would only happen from string dimension indexers of the
incremental index which ultimately always produce string columns with a
dictionary, and these handlers this method are making are used for things like
index merging, etc, and so could safely treat unknown from the dim indexer as
true because it wants the desired state rather than the current state.
Not going to do in this PR, but I am trying to plan out how to refactor
`ColumnCapabilitiesImpl` into a builder that requires a coercion logic to build
into a `ColumnCapabilities`, and changing `ColumnCapabilities` back to having
only `boolean`, instead of the `Capable` enum. The idea being to limit the
surface area of having to deal with unknown to where is absolutely necessary,
because it makes things overly complex to have to deal with these tri-state
booleans everywhere.
----------------------------------------------------------------
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]
