clintropolis commented on a change in pull request #10219:
URL: https://github.com/apache/druid/pull/10219#discussion_r469537834
##########
File path:
processing/src/main/java/org/apache/druid/query/metadata/SegmentAnalyzer.java
##########
@@ -192,6 +193,7 @@ private ColumnAnalysis analyzeNumericColumn(
return new ColumnAnalysis(
capabilities.getType().name(),
capabilities.hasMultipleValues().isTrue(),
+ capabilities.isNullable().isTrue(),
Review comment:
I think maybe the only current unknowns will come from complex columns,
which thinking about it should probably just report that they have nulls as
true instead of unknown...
Right now nothing bad would happen if the segment metadata falsely reports
not having nulls, but in the follow-up where this is wired into `DruidSchema`,
falsely reporting nulls would cause the SQL planner to incorrectly plan to not
handle nulls, so I think this should probably be modified to check
`isMaybeTrue`.
----------------------------------------------------------------
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]
