ChristinaTech opened a new pull request, #7718:
URL: https://github.com/apache/iceberg/pull/7718

   Calling `hasNext()` on the any Iterator returned by the Iterable from 
`SnapshotUtil.ancestorsOf` throws a NullPointerException if it is called again 
after the prior call returned false. This only happens if the ancestor chain is 
broken by an expired snapshot, not if it reaches a snapshot with a null 
parentId.
   
   This is because when `next.parentId()` returns null the iterator will return 
false without changing its state, but when the lookup function returns null, as 
it would when the snapshot's parent is expired, it sets the `next` variable to 
null as well. The first time this doesn't cause an issue as the next line 
null-checks it, but if `hasNext()` is called again it will attempt to call 
`next.parentId()` on the now-null next variable, throwing an NPE.
   
   I considered fixing this by storing the result of `lookup.apply` in an 
intermediate variable and null-checking it before assigning it to `next`, but 
if for any reason the lookup function wasn't deterministic this could cause 
`hasNext()` to return true after it has already returned false, which would be 
a separate violation of the Iterator API contract from the one this PR aims to 
fix. Alternatively, if lookup were expensive it would just be wasted effort to 
call it again.
   
   I added a new test case that replicated the bug in order to validate my 
change fixed it, and in the process did some refactoring in the unit test to 
reduce duplicate code and (at least attempted to) give the snapshot id 
variables more intuitive names. Also added additional assertions to 
`isParentAncestorOf` and `isAncestorOf` tests to validate they work as intended 
with expired snapshots as well.


-- 
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