pvary commented on code in PR #13329:
URL: https://github.com/apache/iceberg/pull/13329#discussion_r2161134653
##########
core/src/main/java/org/apache/iceberg/PartitionStatsHandler.java:
##########
@@ -280,6 +273,8 @@ private static Collection<PartitionStats>
computeAndMergeStatsIncremental(
oldStats.forEach(
partitionStats ->
statsMap.put(partitionStats.specId(),
partitionStats.partition(), partitionStats));
+ } catch (IllegalArgumentException | IllegalStateException exception) {
Review Comment:
My thoughts:
- I would like to see this in 1.10.0 - I care about his more than anything
else. I would accept really any compromise, but we need this out in 1.10.0, so
no more new corrupted stats files are generated.
- I would prefer the extra code which handles corrupted stats files as
simple as possible, as I would like to remove it in 1.11.0, and don't keep it
any longer
- Catching any exception and assuming that is caused by the wrong schema
seems problematic. Almost as problematic as parsing the exception messages.
- I don't see any fool-proof solution to detect this corruption than trying
to read the stat file with the corrupted schema and successfully reading
through it. OTOH, I'm not sure that we want to go this deep with the temp fix.
Also, in this case we might just return this stats instead of throwing an
exception.
@ajantha-bhat: Do we have an exception when we call `hasNext()`? This way we
could test the iterator without changing the state.
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
