amogh-jahagirdar commented on code in PR #18598:
URL: https://github.com/apache/iceberg/pull/18598#discussion_r4418818517
##########
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:
My only other comment here is why not just use primitive boolean and return
false by default?
##########
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:
Yeah I think it makes sense to expose this at least for reads; an engine
that wants to do some forms of aggregation pushdown (e..g min(col)) would need
to be able to determine if the file has tight bounds and this is the main
interface task consumers would be able to look at.
I just wanted to think through if this should be on ContentFile or DataFile
itself. My ultimate conclusion is ContentFile is the right place, but my
thinking is the following:
Whether it makes sense to be on ContentFile, can be answered by if it makes
sense on both DataFile and DeleteFile in the implementation.
Data/Delete will be combined entry in v4, but conceptually from an
implementation perspective I think it fits in ContentFile. We very well could
have v2/v3 Delete files in-memory representation implementations be able to
answer if the stats indicating deleted rows are tight. The reference
implementation doesn't currently produce stats for DVs/position deletes but the
spec doesn't prohibit that and some implementation of ContentFile somewhere may
choose at runtime to surface DVs that it knows has stats which have tight
bounds. So TLDR, conceptually I think this API here makes sense.
--
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]
