[GitHub] [druid] clintropolis commented on issue #8433: StringDictionaryEncodedColumn dimSelector to return CARDINALITY_UNKNOWN with extractionFn

[GitBox <gi...@apache.org>]

clintropolis commented on issue #8433: StringDictionaryEncodedColumn dimSelector to return CARDINALITY_UNKNOWN with extractionFn
URL: https://github.com/apache/druid/pull/8433#issuecomment-579955558
 
 
   >Hmm, I came across this patch while looking at the javadocs for DimensionSelector#getValueCardinality. It doesn't match my understanding, which is that the ids for a selector don't have to be ordered (the ids for a 'real' column have to be, but that's different, and mostly only matters for filtering, which uses a different BitmapIndexSelector interface).
   
   I dug into this a bit because I wasn't sure and I think you are correct, unless I missed something. It appears to me that everywhere that is using this method and expecting ordering have other guards to ensure that is true, either checking `ColumnCapabilities.dimensionValuesSorted`, `DimensionSpec.preservesOrdering`, or `DimensionDictionarySelector.nameLookupPossibleInAdvance` which afaict effectively is the `isDictionarySorted` method you were suggesting.
   
   >If not, I suggest we update DimensionSelector javadocs to say that the ids are not guaranteed to be sorted.
   
   sgtm :+1:

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org