amogh-jahagirdar commented on code in PR #18598:
URL: https://github.com/apache/iceberg/pull/18598#discussion_r4445465307
##########
core/src/main/java/org/apache/iceberg/TrackedFile.java:
##########
@@ -163,6 +167,9 @@ static Schema readSchema(Types.StructType partitionType,
Types.StructType conten
/** Returns the content stats for this entry. */
ContentStats contentStats();
+ /** Returns whether the file's statistics are tight, or null if not set. */
+ Boolean tightStats();
Review Comment:
Should this also be primitive?
##########
core/src/test/java/org/apache/iceberg/TestTrackedFileAdapters.java:
##########
@@ -667,10 +675,12 @@ void dataTrackedFileAdapterFromExistingManifestEntry() {
@Test
void dataTrackedFileAdapterReuse() {
+ DataFile file1 = Mockito.spy(DATA_FILE);
Review Comment:
So the purpose of this test is just to validate that an adapter can be
reused across `wrap()` calls, and looks like it was updated to also validate
that `tightStats` is carried over (and, through the `DelegatingDataFile`
wrapper, the `Delegates` override). I think the original purpose of this test
should be kept, but `dataTrackedFileAdapterFromDataFile` should be updated to
wrap a file that reports tight stats. Then the test validates the behavior of
mapping `hasTightStats()` to `tightStats()` and catches the adapter dropping or
hardcoding the flag.
--
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]
