[GitHub] [druid] suneet-s commented on a change in pull request #12345: Introducing a new config to ignore nulls while computing String Cardinality

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

suneet-s commented on a change in pull request #12345:
URL: https://github.com/apache/druid/pull/12345#discussion_r830206928



##########
File path: docs/configuration/index.md
##########
@@ -750,6 +750,7 @@ Prior to version 0.13.0, Druid string columns treated `''` and `null` values as
 |Property|Description|Default|
 |---|---|---|
 |`druid.generic.useDefaultValueForNull`|When set to `true`, `null` values will be stored as `''` for string columns and `0` for numeric columns. Set to `false` to store and query data in SQL compatible mode.|`true`|
+|`druid.generic.ignoreNullsForStringCardinality`|When set to `true`, `null` values will be ignored for cardinality aggregation over string columns. Set to `false` to include `null` values while estimating cardinality of string columns|`false`|

Review comment:
       My thoughts here are that this sort of feature flag is better placed as a QueryContext flag since it is meant to only impact query results. We can then take advantage of #10208 to allow operators to set a default for all queries on the cluster.




-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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