gortiz commented on code in PR #13839: URL: https://github.com/apache/pinot/pull/13839#discussion_r1734327782
########## pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java: ########## @@ -309,7 +309,11 @@ public void deleteSegmentFile() { private String _stopReason = null; private final Semaphore _segBuildSemaphore; private final boolean _isOffHeap; - private final boolean _nullHandlingEnabled; + /** + * Whether null handling is enabled by default. This value is only used if + * {@link Schema#isEnableColumnBasedNullHandling()} is false. + */ + private final boolean _defaultNullHandlingEnabled; Review Comment: In the current code the value of `_defaultNullHandlingEnabled` always comes from table config, so it makes sense to name it as `_tableLevelNullHandlingEnabled` instead. But I think that is a worse name. Whether the value comes from table config or not is not specially relevant here. The important part is what is documented. Imagine that in future we need to change the value of this field (in a test or a new code path). The person that has to apply that change may be concern about breaking the semantics, because then the value will not be the table level nullability. -- 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-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org