[GitHub] [incubator-druid] himanshug commented on a change in pull request #8433: StringDictionaryEncodedColumn dimSelector to return CARDINALITY_UNKNOWN with extractionFn

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

himanshug commented on a change in pull request #8433: StringDictionaryEncodedColumn dimSelector to return CARDINALITY_UNKNOWN with extractionFn
URL: https://github.com/apache/incubator-druid/pull/8433#discussion_r320477172
 
 

 ##########
 File path: processing/src/main/java/org/apache/druid/segment/column/StringDictionaryEncodedColumn.java
 ##########
 @@ -121,7 +121,11 @@ public HistoricalDimensionSelector makeDimensionSelector(
       @Override
       public int getValueCardinality()
       {
-        return getCardinality();
+        if (extractionFn != null) {
+          return CARDINALITY_UNKNOWN;
 
 Review comment:
   @clintropolis thanks for checking.
   
   `ONE_TO_ONE` isn't enough as we also need to honor the ordering (see update to `getValueCardinality()` javadoc, but I realized that `ExtractionFn` has a `preservesOrdering()` method as well ... if both of them are true then we can say cardinality is same.
   
   even after that change, I see many of the tests fail for GroupBy-V1 ... it was working before incorrectly. So, I see that this PR might be problematic for users who had couple of queries working and they stopped working suddenly. so I am removing the "fix" as it causes more nuisance than it fixes. I am leaving only the documentation change in this PR.
   
   I think better fix would be to use a `ExtractionFnVirtualColumn` and remove `DimensionSpec.getExtractionFn()` altogether. ( related https://github.com/apache/incubator-druid/issues/8242 ) 

----------------------------------------------------------------
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