-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-27733][CORE] Upgrade Avro to version 1.10.1 #31232
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
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.
Why do we need this change?
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 is an improvement because Nullable in that class is used to designate class attributes that are nullable (like javax.annotation) and not to refer to nullability of an Avro field.
|
ok to test. |
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
@iemejia, please fill the PR description. |
|
Test build #134193 has finished for PR 31232 at commit
|
Done thanks for the reminder @HyukjinKwon |
|
All green now PTAL |
dongjoon-hyun
left a comment
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.
Thank you for pinging me, @iemejia .
|
@iemejia Thanks for the work! |
cb3b0ce to
6c6b137
Compare
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Test build #134221 has finished for PR 31232 at commit
|
|
Hive error looks like a flake is there a way to re run that CI job/action? |
|
+CC @xkrogen |
6c6b137 to
f0ea522
Compare
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Yay! green again after rebasing. PTAL again |
|
Test build #134260 has finished for PR 31232 at commit
|
gengliangwang
left a comment
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.
LGTM. Again, thanks for the work!
|
Also cc @bozhang2820 |
|
AvroReadBenchmark result. Avro 1.10.1: |
dongjoon-hyun
left a comment
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.
+1, LGTM. Thank you, @iemejia and all!
Merged to master for Apache Spark 3.2.0.
|
@iemejia I added you to the Apache Spark contributor group and assigned SPARK-27733 to you. Thanks! |
|
Thanks to you @dongjoon-hyun and @wangyum! This PR seems 'trivial' but when you have lived through the multi project issues/fixes and dependencies you realize this would not have been possible without everybody's work. |
|
@wangyum just for learning's sake what command do you use to calculate the benchmarks? |
|
spark/external/avro/src/test/scala/org/apache/spark/sql/execution/benchmark/AvroReadBenchmark.scala Lines 29 to 41 in d897825
|
|
Thanks for the ref 👍 @wangyum ! |
### What changes were proposed in this pull request? Update Avro dependency to version 1.10.1 ### Why are the changes needed? To catch up multiple improvements of Avro as well as fix security issues on transitive dependencies. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Since there were no API changes required we just run the tests Closes apache#31232 from iemejia/SPARK-27733-avro-upgrade. Authored-by: Ismaël Mejía <iemejia@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Update Avro dependency to version 1.10.1 To catch up multiple improvements of Avro as well as fix security issues on transitive dependencies. No Since there were no API changes required we just run the tests Closes apache#31232 from iemejia/SPARK-27733-avro-upgrade. Authored-by: Ismaël Mejía <iemejia@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Update Avro dependency to version 1.10.1 To catch up multiple improvements of Avro as well as fix security issues on transitive dependencies. No Since there were no API changes required we just run the tests Closes apache#31232 from iemejia/SPARK-27733-avro-upgrade. Authored-by: Ismaël Mejía <iemejia@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Update Avro dependency to version 1.10.1 To catch up multiple improvements of Avro as well as fix security issues on transitive dependencies. No Since there were no API changes required we just run the tests Closes apache#31232 from iemejia/SPARK-27733-avro-upgrade. Authored-by: Ismaël Mejía <iemejia@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
What changes were proposed in this pull request?
Update Avro dependency to version 1.10.1
Why are the changes needed?
To catch up multiple improvements of Avro as well as fix security issues on transitive dependencies.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Since there were no API changes required we just run the tests