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. This change might be problematic for users who
had couple of queries working and they would stop working suddenly. so I am
removing the "fix" as it causes more problems 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:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
