-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Spark 3.5: Adapt to Spark 3.5.4 #11802
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
||
| // If a column vector is writable or constant, it should override this method and do nothing. | ||
| // See more details at SPARK-50235, SPARK-50463 (Fixed in Spark 3.5.4) | ||
| public void closeIfFreeable() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should probably have an @Overrideand the comment can go inside the method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we cannot use @Override until we upgrade to Spark 3.5.4.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, but I don't think there's any value in adding this method without actually being on Spark 3.5.4. Does that mean you're also upgrading to 3.5.4 as part of this PR (once it's out)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I misunderstood that it requires Spark 3.5.4 😨
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
w/ or w/o @Override won't cause any functional difference.
do you guys want to wait for Spark 3.5.4 to pass the vote, or release Iceberg 1.7.2 ASAP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there's any value in adding this method without actually being on Spark 3.5.4.
@nastra To not surprise users, I suppose iceberg-spark-runtime-3.5_2.12-1.7.2.jar should work for Spark 3.5.x(unfortunately, it's not true for Spark 3.5.3 due to another issue), that means, no matter which version it is built against, it's binary compatible with a serial of versions of Spark 3.5.
In short, the value of this change is, to make the output iceberg-spark-runtime-3.5_2.12-<v>.jar built against Spark 3.5.2, to be compatible with Spark 3.5.4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should probably wait until 3.5.4 is officially out
|
Spark 3.5.4 is out, close and in favor #11731 |

Adapt to Spark 3.5.4, see more details at SPARK-50235, SPARK-50463