gianm commented on a change in pull request #10219:
URL: https://github.com/apache/druid/pull/10219#discussion_r469592335
##########
File path:
processing/src/main/java/org/apache/druid/query/metadata/SegmentAnalyzer.java
##########
@@ -335,6 +339,8 @@ private ColumnAnalysis analyzeComplexColumn(
{
try (final ComplexColumn complexColumn = columnHolder != null ?
(ComplexColumn) columnHolder.getColumn() : null) {
final boolean hasMultipleValues = capabilities != null &&
capabilities.hasMultipleValues().isTrue();
+ // if we don't know for sure, then we should plan to check for nulls
+ final boolean nullable = capabilities != null &&
capabilities.hasNulls().isMaybeTrue();
Review comment:
Could you please rename this to `hasNulls`, for this reason:
https://github.com/apache/druid/pull/10219#discussion_r469598220?
##########
File path:
processing/src/main/java/org/apache/druid/segment/column/ColumnCapabilities.java
##########
@@ -38,6 +38,7 @@
boolean hasSpatialIndexes();
Capable hasMultipleValues();
boolean isFilterable();
+ Capable isNullable();
Review comment:
I do feel that it shouldn't be called `isNullable`. I think we should
avoid the word "nullable" in core Druid to refer to this concept, because one
day, we might want to introduce the SQL-style concept of nullable vs
non-nullable columns. That concept is more of a definition thing than a reality
thing, and will be confusing to introduce to core if we have the reality-thing
using the word "nullable".
Also, if we did introduce it to core, we'd probably want to report both (a
column that is nullable table-wide might not actually have nulls in a specific
segment, and that's useful to know).
----------------------------------------------------------------
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]
