You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "gbloisi-openaire (via GitHub)" <gi...@apache.org> on 2023/09/13 19:06:45 UTC
[GitHub] [spark] gbloisi-openaire opened a new pull request, #42913: [SPARK-45081][SQL][3.4] Encoders.bean does no longer work with read-only properties
gbloisi-openaire opened a new pull request, #42913:
URL: https://github.com/apache/spark/pull/42913
### What changes were proposed in this pull request?
This PR re-enables Encoders.bean to be called against beans having read-only properties, that is properties that have only getters and no setter method. Beans with read only properties are even used in internal tests.
Setter methods of a Java bean encoder are stored within an Option wrapper because they are missing in case of read-only properties. When a java bean has to be initialized, setter methods for the bean properties have to be called: this PR filters out read-only properties from that process.
### Why are the changes needed?
The changes are required to avoid an exception to the thrown by getting the value of a None option object.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
An additional regression test has been added
### Was this patch authored or co-authored using generative AI tooling?
No
@hvanhovell this is 3.4 branch port of [PR-42829](https://github.com/apache/spark/pull/42829)
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #42913: [SPARK-45081][SQL][3.4] Encoders.bean does no longer work with read-only properties
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #42913:
URL: https://github.com/apache/spark/pull/42913#issuecomment-1718480383
cc @hvanhovell
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #42913: [SPARK-45081][SQL][3.4] Encoders.bean does no longer work with read-only properties
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #42913:
URL: https://github.com/apache/spark/pull/42913#discussion_r1329158225
##########
sql/core/src/test/java/test/org/apache/spark/sql/JavaDatasetSuite.java:
##########
@@ -1711,6 +1711,23 @@ public void testEmptyBean() {
Assert.assertEquals(1, df.collectAsList().size());
}
+ public static class ReadOnlyPropertyBean implements Serializable {
+ public boolean isEmpty() {
Review Comment:
We use 2-space indentation. Could you fix the style, @gbloisi-openaire ?
##########
sql/core/src/test/java/test/org/apache/spark/sql/JavaDatasetSuite.java:
##########
@@ -1711,6 +1711,23 @@ public void testEmptyBean() {
Assert.assertEquals(1, df.collectAsList().size());
}
+ public static class ReadOnlyPropertyBean implements Serializable {
+ public boolean isEmpty() {
Review Comment:
We use 2-space indentation even in Java code. Could you fix the style, @gbloisi-openaire ?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gbloisi-openaire commented on a diff in pull request #42913: [SPARK-45081][SQL][3.4] Encoders.bean does no longer work with read-only properties
Posted by "gbloisi-openaire (via GitHub)" <gi...@apache.org>.
gbloisi-openaire commented on code in PR #42913:
URL: https://github.com/apache/spark/pull/42913#discussion_r1329237187
##########
sql/core/src/test/java/test/org/apache/spark/sql/JavaDatasetSuite.java:
##########
@@ -1711,6 +1711,23 @@ public void testEmptyBean() {
Assert.assertEquals(1, df.collectAsList().size());
}
+ public static class ReadOnlyPropertyBean implements Serializable {
+ public boolean isEmpty() {
Review Comment:
I just committed the required changes. Note that [PR-42829](https://github.com/apache/spark/pull/42829) has been already merged.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gbloisi-openaire commented on a diff in pull request #42913: [SPARK-45081][SQL][3.4] Encoders.bean does no longer work with read-only properties
Posted by "gbloisi-openaire (via GitHub)" <gi...@apache.org>.
gbloisi-openaire commented on code in PR #42913:
URL: https://github.com/apache/spark/pull/42913#discussion_r1329237187
##########
sql/core/src/test/java/test/org/apache/spark/sql/JavaDatasetSuite.java:
##########
@@ -1711,6 +1711,23 @@ public void testEmptyBean() {
Assert.assertEquals(1, df.collectAsList().size());
}
+ public static class ReadOnlyPropertyBean implements Serializable {
+ public boolean isEmpty() {
Review Comment:
I just committed the required changes, also for [PR-42634](https://github.com/apache/spark/pull/42829) that is the same patch against master and 3.5.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #42913: [SPARK-45081][SQL][3.4] Encoders.bean does no longer work with read-only properties
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #42913:
URL: https://github.com/apache/spark/pull/42913#issuecomment-1724338976
Merged to branch-3.4 for Apache Spark 3.4.2.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #42913: [SPARK-45081][SQL][3.4] Encoders.bean does no longer work with read-only properties
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #42913:
URL: https://github.com/apache/spark/pull/42913#discussion_r1329254031
##########
sql/core/src/test/java/test/org/apache/spark/sql/JavaDatasetSuite.java:
##########
@@ -1711,6 +1711,23 @@ public void testEmptyBean() {
Assert.assertEquals(1, df.collectAsList().size());
}
+ public static class ReadOnlyPropertyBean implements Serializable {
+ public boolean isEmpty() {
Review Comment:
Thank you for updated. Ya, I know that the original PR is merged already.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun closed pull request #42913: [SPARK-45081][SQL][3.4] Encoders.bean does no longer work with read-only properties
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #42913: [SPARK-45081][SQL][3.4] Encoders.bean does no longer work with read-only properties
URL: https://github.com/apache/spark/pull/42913
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org