amogh-jahagirdar commented on code in PR #18598:
URL: https://github.com/apache/iceberg/pull/18598#discussion_r4428148724


##########
api/src/main/java/org/apache/iceberg/ContentFile.java:
##########
@@ -99,6 +99,13 @@ default String location() {
   /** Returns if collected, map from column ID to value upper bounds, null 
otherwise. */
   Map<Integer, ByteBuffer> upperBounds();
 
+  /**
+   * Returns whether the file's stats are tight, or null if unknown.
+   */
+  default Boolean tightStats() {

Review Comment:
   I'm still honestly a bit skeptical that null is meaningful for consumers 
here. Generally, the API doesn't need to faithfully represent what's on disk. 
We just need to design for how implementations consume it.
   
   I think the immediate need is just knowing if stats are tight: `boolean 
hasTightStats()`. Unknown would primarily be for older metadata where I think 
I'd add a separate `tightStatsKnown()` later for that.
   
   Then the consumption logic would be :
   
   ```
   if (!file.tightStatsKnown()) {
     // determine if stats are tight
   } else if (!file.hasTightStats()) {
     // known not tight
   }
   ```
   
   Imo that's a bit cleaner than returning `Boolean` and dealing with consuming 
null  state from the API for the use cases tight stats would be used for.
   
   I'll let @nastra @rdblue chime in if they have any opinions here.
   



##########
api/src/main/java/org/apache/iceberg/ContentFile.java:
##########
@@ -99,6 +99,13 @@ default String location() {
   /** Returns if collected, map from column ID to value upper bounds, null 
otherwise. */
   Map<Integer, ByteBuffer> upperBounds();
 
+  /**
+   * Returns whether the file's stats are tight, or null if unknown.
+   */
+  default Boolean tightStats() {

Review Comment:
   It's kind of buried in my response but regardless of the return type, I 
think I'd call this `hasTightStats`



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to