You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/10/18 04:11:47 UTC
[GitHub] [iceberg] hililiwei opened a new pull request, #6005: Core: Fix NPE for parent snapshot does not exist
hililiwei opened a new pull request, #6005:
URL: https://github.com/apache/iceberg/pull/6005
Fix NPE for parent snapshot does not exist
--
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
[GitHub] [iceberg] aokolnychyi merged pull request #6005: Core: Fix NPE for parent snapshot does not exist
Posted by GitBox <gi...@apache.org>.
aokolnychyi merged PR #6005:
URL: https://github.com/apache/iceberg/pull/6005
--
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
[GitHub] [iceberg] ajantha-bhat commented on a diff in pull request #6005: Core: Fix NPE for parent snapshot does not exist
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on code in PR #6005:
URL: https://github.com/apache/iceberg/pull/6005#discussion_r997754505
##########
core/src/main/java/org/apache/iceberg/util/SnapshotUtil.java:
##########
@@ -72,7 +72,7 @@ public static boolean isAncestorOf(Table table, long ancestorSnapshotId) {
public static boolean isParentAncestorOf(
Table table, long snapshotId, long ancestorParentSnapshotId) {
for (Snapshot snapshot : ancestorsOf(snapshotId, table::snapshot)) {
- if (snapshot.parentId() == ancestorParentSnapshotId) {
+ if (snapshot.parentId() != null && snapshot.parentId() == ancestorParentSnapshotId) {
Review Comment:
Can we add a test case for this scenario 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: 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
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #6005: Core: Fix NPE for parent snapshot does not exist
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #6005:
URL: https://github.com/apache/iceberg/pull/6005#discussion_r998627849
##########
core/src/main/java/org/apache/iceberg/util/SnapshotUtil.java:
##########
@@ -72,7 +72,7 @@ public static boolean isAncestorOf(Table table, long ancestorSnapshotId) {
public static boolean isParentAncestorOf(
Table table, long snapshotId, long ancestorParentSnapshotId) {
for (Snapshot snapshot : ancestorsOf(snapshotId, table::snapshot)) {
- if (snapshot.parentId() == ancestorParentSnapshotId) {
+ if (snapshot.parentId() != null && snapshot.parentId() == ancestorParentSnapshotId) {
Review Comment:
We had tests in incremental scans but the added test suite looks good too.
--
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
[GitHub] [iceberg] hililiwei commented on pull request #6005: Core: Fix NPE for parent snapshot does not exist
Posted by GitBox <gi...@apache.org>.
hililiwei commented on PR #6005:
URL: https://github.com/apache/iceberg/pull/6005#issuecomment-1283192034
> @hililiwei, looks like we started getting test failures. Could you check? I will be able to check in 2 hours as well.
>
> ```
> TestSnapshotUtil > oldestAncestor FAILED
> [367](https://github.com/apache/iceberg/actions/runs/3277597635/jobs/5395173940#step:6:368)
> java.lang.AssertionError: expected:<2> but was:<1>
> [368](https://github.com/apache/iceberg/actions/runs/3277597635/jobs/5395173940#step:6:369)
> at org.junit.Assert.fail(Assert.java:89)
> [369](https://github.com/apache/iceberg/actions/runs/3277597635/jobs/5395173940#step:6:370)
> at org.junit.Assert.failNotEquals(Assert.java:835)
> [370](https://github.com/apache/iceberg/actions/runs/3277597635/jobs/5395173940#step:6:371)
> at org.junit.Assert.assertEquals(Assert.java:647)
> [371](https://github.com/apache/iceberg/actions/runs/3277597635/jobs/5395173940#step:6:372)
> at org.junit.Assert.assertEquals(Assert.java:633)
> [372](https://github.com/apache/iceberg/actions/runs/3277597635/jobs/5395173940#step:6:373)
> at org.apache.iceberg.util.TestSnapshotUtil.oldestAncestor(TestSnapshotUtil.java:133)
> ```
@aokolnychyi Okay, let me check what went wrong.
--
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
[GitHub] [iceberg] hililiwei commented on a diff in pull request #6005: Core: Fix NPE for parent snapshot does not exist
Posted by GitBox <gi...@apache.org>.
hililiwei commented on code in PR #6005:
URL: https://github.com/apache/iceberg/pull/6005#discussion_r997934218
##########
core/src/main/java/org/apache/iceberg/util/SnapshotUtil.java:
##########
@@ -72,7 +72,7 @@ public static boolean isAncestorOf(Table table, long ancestorSnapshotId) {
public static boolean isParentAncestorOf(
Table table, long snapshotId, long ancestorParentSnapshotId) {
for (Snapshot snapshot : ancestorsOf(snapshotId, table::snapshot)) {
- if (snapshot.parentId() == ancestorParentSnapshotId) {
+ if (snapshot.parentId() != null && snapshot.parentId() == ancestorParentSnapshotId) {
Review Comment:
SnapshotUtil doesn't have a test case class for it. So I created one, adding some additional test cases.
--
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
[GitHub] [iceberg] aokolnychyi commented on pull request #6005: Core: Fix NPE for parent snapshot does not exist
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on PR #6005:
URL: https://github.com/apache/iceberg/pull/6005#issuecomment-1282903102
Thanks, @hililiwei! Thanks for reviewing, @ajantha-bhat @Fokko!
--
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
[GitHub] [iceberg] aokolnychyi commented on pull request #6005: Core: Fix NPE for parent snapshot does not exist
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on PR #6005:
URL: https://github.com/apache/iceberg/pull/6005#issuecomment-1283143823
@hililiwei, looks like we started getting test failures. Could you check? I will be able to check in 2 hours as well.
```
TestSnapshotUtil > oldestAncestor FAILED
[367](https://github.com/apache/iceberg/actions/runs/3277597635/jobs/5395173940#step:6:368)
java.lang.AssertionError: expected:<2> but was:<1>
[368](https://github.com/apache/iceberg/actions/runs/3277597635/jobs/5395173940#step:6:369)
at org.junit.Assert.fail(Assert.java:89)
[369](https://github.com/apache/iceberg/actions/runs/3277597635/jobs/5395173940#step:6:370)
at org.junit.Assert.failNotEquals(Assert.java:835)
[370](https://github.com/apache/iceberg/actions/runs/3277597635/jobs/5395173940#step:6:371)
at org.junit.Assert.assertEquals(Assert.java:647)
[371](https://github.com/apache/iceberg/actions/runs/3277597635/jobs/5395173940#step:6:372)
at org.junit.Assert.assertEquals(Assert.java:633)
[372](https://github.com/apache/iceberg/actions/runs/3277597635/jobs/5395173940#step:6:373)
at org.apache.iceberg.util.TestSnapshotUtil.oldestAncestor(TestSnapshotUtil.java:133)
```
--
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
[GitHub] [iceberg] hililiwei commented on pull request #6005: Core: Fix NPE for parent snapshot does not exist
Posted by GitBox <gi...@apache.org>.
hililiwei commented on PR #6005:
URL: https://github.com/apache/iceberg/pull/6005#issuecomment-1283235500
@aokolnychyi #6015 try to fix 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