flyrain commented on PR #11612: URL: https://github.com/apache/iceberg/pull/11612#issuecomment-2568621811
> 2. `endSnapshot` is `null` (`endTimestamp == null` ensures this is from calculation) Makes sense to check 2 within `if (startTimestamp != null || endTimestamp != null)` with the refactor. However as I said [here](https://github.com/apache/iceberg/pull/11612#discussion_r1901380346), I don't recommend the refactor(line615-618), which was introducing the checking 2. > 1. both `startSnapshot` and `endSnapshot` are `null` > 3. `startSnapshot` equals to `endSnapshot` Moreover, we may still check 1 and 3 out side of `if (startTimestamp != null || endTimestamp != null)`, as even start snapshot id and end snapshot id are NOT calculated from timestamps, they could be both null or the same. Am I miss something? -- 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
