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

Reply via email to