You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/04/22 14:57:22 UTC
[GitHub] [spark] rashtao opened a new pull request #32299: Allow custom JsonGenerator in JacksonGenerator
rashtao opened a new pull request #32299:
URL: https://github.com/apache/spark/pull/32299
This PR provides an overloaded version of `JacksonGenerator` constructor that accepts a custom `JsonGenerator` instead of `java.io.Writer`. This would enable the use of `JacksonGenerator` with any [Jackson binary dataformat](https://github.com/FasterXML/jackson-dataformats-binary).
Those Jackson implementations mostly do not have support for `com.fasterxml.jackson.core.JsonFactory#createGenerator(java.io.Writer)`, but only for `com.fasterxml.jackson.core.JsonFactory#createGenerator(java.io.OutputStream)`.
### Why are the changes needed?
This would be beneficial for datasources implementations, which need to convert data to any Jackson binary dataformat, since they could reuse `JacksonGenerator` type mappings.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
I added a positive unit test.
--
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.
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 a change in pull request #32299: Allow custom JsonGenerator in JacksonGenerator
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32299:
URL: https://github.com/apache/spark/pull/32299#discussion_r619073474
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/json/JacksonGenerator.scala
##########
@@ -37,8 +37,18 @@ import org.apache.spark.sql.types._
*/
private[sql] class JacksonGenerator(
Review comment:
`JacksonGenerator` isn't an API so it doesn't make much sense to make it pluggable
--
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.
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] github-actions[bot] closed pull request #32299: Allow custom JsonGenerator in JacksonGenerator
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #32299:
URL: https://github.com/apache/spark/pull/32299
--
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] rashtao commented on a change in pull request #32299: Allow custom JsonGenerator in JacksonGenerator
Posted by GitBox <gi...@apache.org>.
rashtao commented on a change in pull request #32299:
URL: https://github.com/apache/spark/pull/32299#discussion_r619150925
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/json/JacksonGenerator.scala
##########
@@ -37,8 +37,18 @@ import org.apache.spark.sql.types._
*/
private[sql] class JacksonGenerator(
Review comment:
As far I can see datasources are sometimes implemented under `org.apache.spark.sql`, thus accessing private and protected members, ie. https://github.com/datastax/spark-cassandra-connector/tree/master/connector/src/main/scala/org/apache/spark/sql/cassandra
Of course relying on private API could lead to unpredictable breaking changes. Ideally `JacksonParser` and `JacksonGenerator` could evolve to a public API usable by any binary dataformat implementation.
--
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.
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] AmplabJenkins commented on pull request #32299: Allow custom JsonGenerator in JacksonGenerator
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32299:
URL: https://github.com/apache/spark/pull/32299#issuecomment-824927657
Can one of the admins verify this patch?
--
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.
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] github-actions[bot] commented on pull request #32299: Allow custom JsonGenerator in JacksonGenerator
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #32299:
URL: https://github.com/apache/spark/pull/32299#issuecomment-890613520
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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