You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "lirui-apache (via GitHub)" <gi...@apache.org> on 2025/05/26 09:22:43 UTC

[I] Incrementally computing partition stats can missing deleted files [iceberg]

lirui-apache opened a new issue, #13155:
URL: https://github.com/apache/iceberg/issues/13155

   ### Apache Iceberg version
   
   main (development)
   
   ### Query engine
   
   None
   
   ### Please describe the bug 🐞
   
   It seems stats diff is computed only from the current snapshot, which may not track deleted files. For example the following test case would fail:
   ```java
     @Test
     public void test() throws Exception {
       Table testTable =
           TestTables.create(tempDir("my_test"), "my_test", SCHEMA, SPEC, 2, fileFormatProperty);
   
       DataFile dataFile1 =
           DataFiles.builder(SPEC)
               .withPath("/df1.parquet")
               .withPartitionPath("c2=a/c3=a")
               .withFileSizeInBytes(10)
               .withRecordCount(1)
               .build();
       DataFile dataFile2 =
           DataFiles.builder(SPEC)
               .withPath("/df2.parquet")
               .withPartitionPath("c2=b/c3=b")
               .withFileSizeInBytes(10)
               .withRecordCount(1)
               .build();
   
       testTable.newAppend().appendFile(dataFile1).appendFile(dataFile2).commit();
   
       testTable
           .updatePartitionStatistics()
           .setPartitionStatistics(PartitionStatsHandler.computeAndWriteStatsFile(testTable))
           .commit();
   
       testTable.newDelete().deleteFile(dataFile1).commit();
       testTable.newDelete().deleteFile(dataFile2).commit();
   
       PartitionStatisticsFile statsFile = PartitionStatsHandler.computeAndWriteStatsFile(testTable);
   
       assertThat(
               PartitionStatsHandler.readPartitionStatsFile(
                   PartitionStatsHandler.schema(Partitioning.partitionType(testTable)),
                   Files.localInput(statsFile.path())))
           .allMatch(s -> s.dataRecordCount() == 0);
     }
   ```
   
   ### Willingness to contribute
   
   - [ ] I can contribute a fix for this bug independently
   - [ ] I would be willing to contribute a fix for this bug with guidance from the Iceberg community
   - [ ] I cannot contribute a fix for this bug at this time


-- 
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: issues-unsubscribe@iceberg.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [I] Incrementally computing partition stats can miss deleted files [iceberg]

Posted by "lirui-apache (via GitHub)" <gi...@apache.org>.
lirui-apache commented on issue #13155:
URL: https://github.com/apache/iceberg/issues/13155#issuecomment-2909728763

   I think when a snapshot is being created, previous manifests without live entries will not be tracked. So maybe we have to apply the incremental snapshots one by one? And probably limit the max number of such snapshots, i.e. if there're too many, a full computation could be a better choice.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [I] Incrementally computing partition stats can miss deleted files [iceberg]

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on issue #13155:
URL: https://github.com/apache/iceberg/issues/13155#issuecomment-2913380025

   I think other option is to just consider DELETED entires from previous snapshots as it won't be carried out for next snapshot. Now I considered all the entries from all the previous snapshots and it gave problem for merge on read deletes.   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [I] Incrementally computing partition stats can miss deleted files [iceberg]

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on issue #13155:
URL: https://github.com/apache/iceberg/issues/13155#issuecomment-2912175962

   > Yeah, given a target snapshot, we can walk through the ancestors and look for an existing stats file. If we can find one, it means all snapshots in between are still there, then we apply them one by one. A possible corner case is an ExpireSnapshot action comes in after we found the ancestors, but before we apply them. I suppose users can retry and fall back to full compute in that case?
   
   Yes, Thats what I have in mind too for this. 
   
   But while fixing this issue, looks like I found a bug in Iceberg implementation, Added the issue in the note (https://github.com/apache/iceberg/pull/13163#issue-3093601428). Debugging it. 
   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [I] Incrementally computing partition stats can miss deleted files [iceberg]

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on issue #13155:
URL: https://github.com/apache/iceberg/issues/13155#issuecomment-2909762512

   > I think when a snapshot is being created, previous manifests without live entries will not be tracked. So maybe we have to apply the incremental snapshots one by one? And probably limit the max number of such snapshots, i.e. if there're too many, a full computation could be a better choice.
   
   We can apply one by one and throw exception if the snapshots removed (expired) in between? So, user does full compute? Thats what make sense to me. 


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [I] Incrementally computing partition stats can miss deleted files [iceberg]

Posted by "lirui-apache (via GitHub)" <gi...@apache.org>.
lirui-apache commented on issue #13155:
URL: https://github.com/apache/iceberg/issues/13155#issuecomment-2909052640

   Hi @ajantha-bhat , could you help verify the issue, thanks!


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [I] Incrementally computing partition stats can miss deleted files [iceberg]

Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary closed issue #13155: Incrementally computing partition stats can miss deleted files
URL: https://github.com/apache/iceberg/issues/13155


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [I] Incrementally computing partition stats can miss deleted files [iceberg]

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on issue #13155:
URL: https://github.com/apache/iceberg/issues/13155#issuecomment-2909461143

   Hi, @lirui-apache thanks for reporting this. 
   
   Looks like this is an edge case with copy-on-write delete. (Test cases covers only merge on read!). 
   
   I assumed that current snapshots track/reuse the manifests from previous snapshot. So, that `PartitionStatsHandler.computeStats` will include previous manifests as the predicate is to include manifests with older snapshot ids. 
   
   But in this case `snapshot.allmanifests()` gives only one manifest file with second delete. 


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [I] Incrementally computing partition stats can miss deleted files [iceberg]

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on issue #13155:
URL: https://github.com/apache/iceberg/issues/13155#issuecomment-2916012247

   I found a clean way. Existing testacase was failing because I included same manifest twice when reused. Now, I added a snapshot id check. So, each snapshot only considers the manifest added by it. 
   
   The PR is ready for review. 


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [I] Incrementally computing partition stats can miss deleted files [iceberg]

Posted by "lirui-apache (via GitHub)" <gi...@apache.org>.
lirui-apache commented on issue #13155:
URL: https://github.com/apache/iceberg/issues/13155#issuecomment-2909799838

   > > I think when a snapshot is being created, previous manifests without live entries will not be tracked. So maybe we have to apply the incremental snapshots one by one? And probably limit the max number of such snapshots, i.e. if there're too many, a full computation could be a better choice.
   > 
   > We can apply one by one and throw exception if the snapshots removed (expired) in between? So, user does full compute? Thats what make sense to me.
   
   Yeah, given a target snapshot, we can walk through the ancestors and look for an existing stats file. If we can find one, it means all snapshots in between are still there, then we apply them one by one. A possible corner case is an ExpireSnapshot action comes in after we found the ancestors, but before we apply them. I suppose users can retry and fall back to full compute in that case?


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org