Skip to content

Conversation

@dramaticlly
Copy link
Contributor

This fix help throw proper validation exception instead of NullPointerException when create branch or tag on iceberg table without any snapshot.

Before

Before
```scala
scala> val snapshotId = branchOptions.snapshotId.getOrElse(table.currentSnapshot().snapshotId())
java.lang.NullPointerException
  at $anonfun$res38$1(<console>:47)
  at scala.runtime.java8.JFunction0$mcJ$sp.apply(JFunction0$mcJ$sp.java:23)
  at scala.Option.getOrElse(Option.scala:189)
  ... 51 elided

After

scala> val snapshotId: java.lang.Long = branchOptions.snapshotId
          .orElse(Option(iceberg.table.currentSnapshot()).map(_.snapshotId()))
          .map(java.lang.Long.valueOf)
          .orNull
snapshotId: Long = null

CC @amogh-jahagirdar @zhangbutao

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for fixing this @dramaticlly

@Fokko Fokko added this to the Iceberg 1.3.0 milestone May 19, 2023
@Fokko
Copy link
Contributor

Fokko commented May 19, 2023

@amogh-jahagirdar Would be great to get this into 1.3.0 as well

Copy link
Contributor

@amogh-jahagirdar amogh-jahagirdar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dramaticlly just a comment on the exception message. Agree @Fokko woul be great to get this in!

Copy link
Contributor

@amogh-jahagirdar amogh-jahagirdar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dramaticlly ! Will merge when CI completes

@zhangbutao
Copy link
Contributor

zhangbutao commented May 21, 2023

I think this fix is the same idea I had before #7593. But it doesn't matter, I am glad to see it was fixed. Thanks. closing my previous pr.

@dramaticlly dramaticlly deleted the branchDDL branch June 28, 2023 21:43
manuzhang added a commit to manuzhang/iceberg that referenced this pull request Aug 3, 2023
This is based on apache#7097, and back-ports bug fixes apache#7652 and apache#8125
manuzhang added a commit to manuzhang/iceberg that referenced this pull request Aug 3, 2023
This is based on apache#7097, and back-ports bug fixes apache#7652 and apache#8086
nastra pushed a commit that referenced this pull request Aug 3, 2023
This is based on #7097, and back-ports bug fixes #7652 and #8086
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants