You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "amaliujia (via GitHub)" <gi...@apache.org> on 2023/06/01 18:11:41 UTC
[GitHub] [spark] amaliujia opened a new pull request, #41425: [SPARK-43919] Extract JSON functionality out of Row
amaliujia opened a new pull request, #41425:
URL: https://github.com/apache/spark/pull/41425
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'core/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
Extract JSON functionality out of Row. Row is public API that is needed by Spark Connect client. We are planning to move Row to a shared module. However Json functionality is unstable API that is not needed by the client. So we can leave that within catalyst module.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
Towards Spark Connect client not depend on catalyst and core.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
Existing UT
--
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
Re: [PR] [SPARK-43919][SQL] Extract JSON functionality out of Row [spark]
Posted by "tfinn-ias (via GitHub)" <gi...@apache.org>.
tfinn-ias commented on PR #41425:
URL: https://github.com/apache/spark/pull/41425#issuecomment-1893858562
Hi, your current public documentation for the Java API lists these methods as available with no disclaimers about them being private or unstable:
https://spark.apache.org/docs/3.4.1/api/java/index.html
--
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] LuciferYang commented on a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1213861276
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/util/ToJsonUtil.scala:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.util
+
+import java.sql.{Date, Timestamp}
+import java.time.{Instant, LocalDate}
+import java.util.Base64
+
+import scala.collection.mutable
+
+import org.json4s._
+import org.json4s.JsonAST.JValue
+import org.json4s.jackson.JsonMethods.{compact, pretty, render}
+
+import org.apache.spark.annotation.Unstable
+import org.apache.spark.sql.Row
+import org.apache.spark.sql.catalyst.CatalystTypeConverters
+import org.apache.spark.sql.catalyst.util.{DateFormatter, DateTimeUtils, TimestampFormatter}
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types._
+import org.apache.spark.unsafe.types.CalendarInterval
+
+
+object ToJsonUtil {
+ /**
+ * The compact JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def json(row: Row): String = compact(jsonValue(row))
+
+ /**
+ * The pretty (i.e. indented) JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def prettyJson(row: Row): String = pretty(render(jsonValue(row)))
+
+ /**
+ * JSON representation of the row.
+ *
+ * Note that this only supports the data types that are also supported by
+ * [[org.apache.spark.sql.catalyst.encoders.RowEncoder]].
+ *
+ * @return the JSON representation of the row.
+ */
+ private[sql] def jsonValue(row: Row): JValue = {
+ require(row.schema != null, "JSON serialization requires a non-null schema.")
+
+ lazy val zoneId = DateTimeUtils.getZoneId(SQLConf.get.sessionLocalTimeZone)
+ lazy val dateFormatter = DateFormatter()
+ lazy val timestampFormatter = TimestampFormatter(zoneId)
+
+ // Convert an iterator of values to a json array
+ def iteratorToJsonArray(iterator: Iterator[_], elementType: DataType): JArray = {
+ JArray(iterator.map(toJson(_, elementType)).toList)
+ }
+
+ // Convert a value to json.
+ def toJson(value: Any, dataType: DataType): JValue = (value, dataType) match {
Review Comment:
Perhaps we can use the Jackson API to rewrite this logic to weaken our dependence on json4s, which could be another pr.
--
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] cloud-fan commented on pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #41425:
URL: https://github.com/apache/spark/pull/41425#issuecomment-1575992736
@amaliujia can you fix conflicts? I think this PR is ready to go
--
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 a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1213959741
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/Row.scala:
##########
@@ -526,95 +513,4 @@ trait Row extends Serializable {
private def getAnyValAs[T <: AnyVal](i: Int): T =
if (isNullAt(i)) throw QueryExecutionErrors.valueIsNullError(i)
else getAs[T](i)
-
- /**
- * The compact JSON representation of this row.
- * @since 3.0
- */
- @Unstable
- def json: String = compact(jsonValue)
Review Comment:
Actually I think we shouldn't remove this ... we can deprecate them.
--
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] cloud-fan commented on pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #41425:
URL: https://github.com/apache/spark/pull/41425#issuecomment-1576919777
```
[error] /home/runner/work/spark/spark/project/MimaExcludes.scala:55:19: ')' expected but '.' found.
[error] ProblemFilters.exclude[ReversedMissingMethodProblem]("org.apache.spark.sql.catalog.Catalog.listDatabases")
[error] ^
[error] /home/runner/work/spark/spark/project/MimaExcludes.scala:56:3: ';' expected but ')' found.
[error] )
[error] ^
[error] two errors found
```
--
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] hvanhovell closed pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell closed pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
URL: https://github.com/apache/spark/pull/41425
--
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] hvanhovell commented on a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1214074665
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/Row.scala:
##########
@@ -526,95 +513,4 @@ trait Row extends Serializable {
private def getAnyValAs[T <: AnyVal](i: Int): T =
if (isNullAt(i)) throw QueryExecutionErrors.valueIsNullError(i)
else getAs[T](i)
-
- /**
- * The compact JSON representation of this row.
- * @since 3.0
- */
- @Unstable
- def json: String = compact(jsonValue)
Review Comment:
@HyukjinKwon I think it is OK to remove since it is unstable. My main motivation for removing it is that it will significantly reduce the amount of code we need to move to the sql/api module, and that it needs SQLConfs to properly function.
--
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
Re: [PR] [SPARK-43919][SQL] Extract JSON functionality out of Row [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #41425:
URL: https://github.com/apache/spark/pull/41425#issuecomment-1898495384
@tfinn-ias so it is a Databricks problem? The `toJsonValue` function is available in SPARK-3.4. In general it is not recommended to use methods that are marked as `private[sql]` since this is not part of the public API. We can change this in a future release, and vendors are free to change this.
If it is a Databricks problem then I'd suggest that you take it up with them. For transparency I work at Databricks but in this case we should go through the formal channels.
--
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
Re: [PR] [SPARK-43919][SQL] Extract JSON functionality out of Row [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #41425:
URL: https://github.com/apache/spark/pull/41425#issuecomment-1893877733
@tfinn-ias what is the question? We put the toJson functionality back in a later PR.
--
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] LuciferYang commented on a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1213859516
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/util/ToJsonUtil.scala:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.util
+
+import java.sql.{Date, Timestamp}
+import java.time.{Instant, LocalDate}
+import java.util.Base64
+
+import scala.collection.mutable
+
+import org.json4s._
+import org.json4s.JsonAST.JValue
+import org.json4s.jackson.JsonMethods.{compact, pretty, render}
+
+import org.apache.spark.annotation.Unstable
+import org.apache.spark.sql.Row
+import org.apache.spark.sql.catalyst.CatalystTypeConverters
+import org.apache.spark.sql.catalyst.util.{DateFormatter, DateTimeUtils, TimestampFormatter}
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types._
+import org.apache.spark.unsafe.types.CalendarInterval
+
+
+object ToJsonUtil {
+ /**
+ * The compact JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def json(row: Row): String = compact(jsonValue(row))
+
+ /**
+ * The pretty (i.e. indented) JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def prettyJson(row: Row): String = pretty(render(jsonValue(row)))
+
+ /**
+ * JSON representation of the row.
+ *
+ * Note that this only supports the data types that are also supported by
+ * [[org.apache.spark.sql.catalyst.encoders.RowEncoder]].
+ *
+ * @return the JSON representation of the row.
+ */
+ private[sql] def jsonValue(row: Row): JValue = {
+ require(row.schema != null, "JSON serialization requires a non-null schema.")
+
+ lazy val zoneId = DateTimeUtils.getZoneId(SQLConf.get.sessionLocalTimeZone)
+ lazy val dateFormatter = DateFormatter()
+ lazy val timestampFormatter = TimestampFormatter(zoneId)
+
+ // Convert an iterator of values to a json array
+ def iteratorToJsonArray(iterator: Iterator[_], elementType: DataType): JArray = {
+ JArray(iterator.map(toJson(_, elementType)).toList)
+ }
+
+ // Convert a value to json.
+ def toJson(value: Any, dataType: DataType): JValue = (value, dataType) match {
Review Comment:
should we make this function to `private[sql]`, `JValue` is a definition in json4s, and I think we should try to avoid exposing it to users in the public api
--
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] cloud-fan commented on a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1217703384
##########
docs/sql-migration-guide.md:
##########
@@ -27,6 +27,7 @@ license: |
- Since Spark 3.5, the JDBC options related to DS V2 pushdown are `true` by default. These options include: `pushDownAggregate`, `pushDownLimit`, `pushDownOffset` and `pushDownTableSample`. To restore the legacy behavior, please set them to `false`. e.g. set `spark.sql.catalog.your_catalog_name.pushDownAggregate` to `false`.
- Since Spark 3.5, Spark thrift server will interrupt task when canceling a running statement. To restore the previous behavior, set `spark.sql.thriftServer.interruptOnCancel` to `false`.
- Since Spark 3.5, the Avro will throw `AnalysisException` when reading Interval types as Date or Timestamp types, or reading Decimal types with lower precision. To restore the legacy behavior, set `spark.sql.legacy.avro.allowIncompatibleSchema` to `true`
+- Since Spark 3.5, Row's json and prettyJson methods are moved to `ToJsonUtil` and also are deprecated.
Review Comment:
```suggestion
- Since Spark 3.5, Row's json and prettyJson methods are moved to `ToJsonUtil`.
```
--
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 a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1214083486
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/Row.scala:
##########
@@ -526,95 +513,4 @@ trait Row extends Serializable {
private def getAnyValAs[T <: AnyVal](i: Int): T =
if (isNullAt(i)) throw QueryExecutionErrors.valueIsNullError(i)
else getAs[T](i)
-
- /**
- * The compact JSON representation of this row.
- * @since 3.0
- */
- @Unstable
- def json: String = compact(jsonValue)
Review Comment:
👌
--
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 a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1213959325
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/Row.scala:
##########
@@ -526,95 +513,4 @@ trait Row extends Serializable {
private def getAnyValAs[T <: AnyVal](i: Int): T =
if (isNullAt(i)) throw QueryExecutionErrors.valueIsNullError(i)
else getAs[T](i)
-
- /**
- * The compact JSON representation of this row.
- * @since 3.0
- */
- @Unstable
- def json: String = compact(jsonValue)
Review Comment:
Should add an item at https://github.com/apache/spark/blob/master/docs/sql-migration-guide.md
--
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] amaliujia commented on a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1213969570
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/util/ToJsonUtil.scala:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.util
+
+import java.sql.{Date, Timestamp}
+import java.time.{Instant, LocalDate}
+import java.util.Base64
+
+import scala.collection.mutable
+
+import org.json4s._
+import org.json4s.JsonAST.JValue
+import org.json4s.jackson.JsonMethods.{compact, pretty, render}
+
+import org.apache.spark.annotation.Unstable
+import org.apache.spark.sql.Row
+import org.apache.spark.sql.catalyst.CatalystTypeConverters
+import org.apache.spark.sql.catalyst.util.{DateFormatter, DateTimeUtils, TimestampFormatter}
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types._
+import org.apache.spark.unsafe.types.CalendarInterval
+
+
+object ToJsonUtil {
+ /**
+ * The compact JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def json(row: Row): String = compact(jsonValue(row))
+
+ /**
+ * The pretty (i.e. indented) JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def prettyJson(row: Row): String = pretty(render(jsonValue(row)))
+
+ /**
+ * JSON representation of the row.
+ *
+ * Note that this only supports the data types that are also supported by
+ * [[org.apache.spark.sql.catalyst.encoders.RowEncoder]].
+ *
+ * @return the JSON representation of the row.
+ */
+ private[sql] def jsonValue(row: Row): JValue = {
+ require(row.schema != null, "JSON serialization requires a non-null schema.")
+
+ lazy val zoneId = DateTimeUtils.getZoneId(SQLConf.get.sessionLocalTimeZone)
+ lazy val dateFormatter = DateFormatter()
+ lazy val timestampFormatter = TimestampFormatter(zoneId)
+
+ // Convert an iterator of values to a json array
+ def iteratorToJsonArray(iterator: Iterator[_], elementType: DataType): JArray = {
+ JArray(iterator.map(toJson(_, elementType)).toList)
+ }
+
+ // Convert a value to json.
+ def toJson(value: Any, dataType: DataType): JValue = (value, dataType) match {
Review Comment:
> Perhaps we can use the Jackson API to rewrite this logic to weaken our dependence on json4s, which could be another pr (https://issues.apache.org/jira/browse/SPARK-39658 recorded this, but did not start discussing).
+1
--
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] LuciferYang commented on a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1213861276
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/util/ToJsonUtil.scala:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.util
+
+import java.sql.{Date, Timestamp}
+import java.time.{Instant, LocalDate}
+import java.util.Base64
+
+import scala.collection.mutable
+
+import org.json4s._
+import org.json4s.JsonAST.JValue
+import org.json4s.jackson.JsonMethods.{compact, pretty, render}
+
+import org.apache.spark.annotation.Unstable
+import org.apache.spark.sql.Row
+import org.apache.spark.sql.catalyst.CatalystTypeConverters
+import org.apache.spark.sql.catalyst.util.{DateFormatter, DateTimeUtils, TimestampFormatter}
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types._
+import org.apache.spark.unsafe.types.CalendarInterval
+
+
+object ToJsonUtil {
+ /**
+ * The compact JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def json(row: Row): String = compact(jsonValue(row))
+
+ /**
+ * The pretty (i.e. indented) JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def prettyJson(row: Row): String = pretty(render(jsonValue(row)))
+
+ /**
+ * JSON representation of the row.
+ *
+ * Note that this only supports the data types that are also supported by
+ * [[org.apache.spark.sql.catalyst.encoders.RowEncoder]].
+ *
+ * @return the JSON representation of the row.
+ */
+ private[sql] def jsonValue(row: Row): JValue = {
+ require(row.schema != null, "JSON serialization requires a non-null schema.")
+
+ lazy val zoneId = DateTimeUtils.getZoneId(SQLConf.get.sessionLocalTimeZone)
+ lazy val dateFormatter = DateFormatter()
+ lazy val timestampFormatter = TimestampFormatter(zoneId)
+
+ // Convert an iterator of values to a json array
+ def iteratorToJsonArray(iterator: Iterator[_], elementType: DataType): JArray = {
+ JArray(iterator.map(toJson(_, elementType)).toList)
+ }
+
+ // Convert a value to json.
+ def toJson(value: Any, dataType: DataType): JValue = (value, dataType) match {
Review Comment:
Perhaps we can use the Jackson API to rewrite this logic to weaken our dependence on json4s, which could be another pr (https://issues.apache.org/jira/browse/SPARK-39658 recorded this, but did not start discussing).
--
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] LuciferYang commented on a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1213859516
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/util/ToJsonUtil.scala:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.util
+
+import java.sql.{Date, Timestamp}
+import java.time.{Instant, LocalDate}
+import java.util.Base64
+
+import scala.collection.mutable
+
+import org.json4s._
+import org.json4s.JsonAST.JValue
+import org.json4s.jackson.JsonMethods.{compact, pretty, render}
+
+import org.apache.spark.annotation.Unstable
+import org.apache.spark.sql.Row
+import org.apache.spark.sql.catalyst.CatalystTypeConverters
+import org.apache.spark.sql.catalyst.util.{DateFormatter, DateTimeUtils, TimestampFormatter}
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types._
+import org.apache.spark.unsafe.types.CalendarInterval
+
+
+object ToJsonUtil {
+ /**
+ * The compact JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def json(row: Row): String = compact(jsonValue(row))
+
+ /**
+ * The pretty (i.e. indented) JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def prettyJson(row: Row): String = pretty(render(jsonValue(row)))
+
+ /**
+ * JSON representation of the row.
+ *
+ * Note that this only supports the data types that are also supported by
+ * [[org.apache.spark.sql.catalyst.encoders.RowEncoder]].
+ *
+ * @return the JSON representation of the row.
+ */
+ private[sql] def jsonValue(row: Row): JValue = {
+ require(row.schema != null, "JSON serialization requires a non-null schema.")
+
+ lazy val zoneId = DateTimeUtils.getZoneId(SQLConf.get.sessionLocalTimeZone)
+ lazy val dateFormatter = DateFormatter()
+ lazy val timestampFormatter = TimestampFormatter(zoneId)
+
+ // Convert an iterator of values to a json array
+ def iteratorToJsonArray(iterator: Iterator[_], elementType: DataType): JArray = {
+ JArray(iterator.map(toJson(_, elementType)).toList)
+ }
+
+ // Convert a value to json.
+ def toJson(value: Any, dataType: DataType): JValue = (value, dataType) match {
Review Comment:
should we make this function to `private[sql]`, `JValue` is a definition in json4s, and we should try to avoid exposing it to users in the public api
--
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] amaliujia commented on a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1213945590
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/Row.scala:
##########
@@ -526,95 +513,4 @@ trait Row extends Serializable {
private def getAnyValAs[T <: AnyVal](i: Int): T =
if (isNullAt(i)) throw QueryExecutionErrors.valueIsNullError(i)
else getAs[T](i)
-
- /**
- * The compact JSON representation of this row.
- * @since 3.0
- */
- @Unstable
- def json: String = compact(jsonValue)
Review Comment:
@cloud-fan is there an example of adding migration guide?
--
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] amaliujia commented on pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on PR #41425:
URL: https://github.com/apache/spark/pull/41425#issuecomment-1576068930
@cloud-fan done. wait CI to pass again.
--
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
Re: [PR] [SPARK-43919][SQL] Extract JSON functionality out of Row [spark]
Posted by "tfinn-ias (via GitHub)" <gi...@apache.org>.
tfinn-ias commented on PR #41425:
URL: https://github.com/apache/spark/pull/41425#issuecomment-1898540795
So I think there's a Spark problem in that the official Java documentation does not reflect the fact that methods are `private[sql]` or `@Unstable` which will deceive people into thinking things are supported that are not (e.g. how are users using Spark from Java supposed to tell from this page that `jsonValue` is not part of the public API? https://spark.apache.org/docs/3.4.1/api/java/org/apache/spark/sql/Row.html ).
But the rest seems to be a Databricks problem, following up with our assigned databricks consultant internally on their version of Spark 3.4.1 being different to the public version.
--
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] amaliujia commented on a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1213967613
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/util/ToJsonUtil.scala:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.util
+
+import java.sql.{Date, Timestamp}
+import java.time.{Instant, LocalDate}
+import java.util.Base64
+
+import scala.collection.mutable
+
+import org.json4s._
+import org.json4s.JsonAST.JValue
+import org.json4s.jackson.JsonMethods.{compact, pretty, render}
+
+import org.apache.spark.annotation.Unstable
+import org.apache.spark.sql.Row
+import org.apache.spark.sql.catalyst.CatalystTypeConverters
+import org.apache.spark.sql.catalyst.util.{DateFormatter, DateTimeUtils, TimestampFormatter}
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types._
+import org.apache.spark.unsafe.types.CalendarInterval
+
+
+object ToJsonUtil {
+ /**
+ * The compact JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def json(row: Row): String = compact(jsonValue(row))
+
+ /**
+ * The pretty (i.e. indented) JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def prettyJson(row: Row): String = pretty(render(jsonValue(row)))
+
+ /**
+ * JSON representation of the row.
+ *
+ * Note that this only supports the data types that are also supported by
+ * [[org.apache.spark.sql.catalyst.encoders.RowEncoder]].
+ *
+ * @return the JSON representation of the row.
+ */
+ private[sql] def jsonValue(row: Row): JValue = {
+ require(row.schema != null, "JSON serialization requires a non-null schema.")
+
+ lazy val zoneId = DateTimeUtils.getZoneId(SQLConf.get.sessionLocalTimeZone)
+ lazy val dateFormatter = DateFormatter()
+ lazy val timestampFormatter = TimestampFormatter(zoneId)
+
+ // Convert an iterator of values to a json array
+ def iteratorToJsonArray(iterator: Iterator[_], elementType: DataType): JArray = {
+ JArray(iterator.map(toJson(_, elementType)).toList)
+ }
+
+ // Convert a value to json.
+ def toJson(value: Any, dataType: DataType): JValue = (value, dataType) match {
Review Comment:
this is within `private[sql] def jsonValue` so I assumed it is already `private[sql]`? Do I misunderstand Scala here?
--
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] LuciferYang commented on a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1213969022
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/util/ToJsonUtil.scala:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.util
+
+import java.sql.{Date, Timestamp}
+import java.time.{Instant, LocalDate}
+import java.util.Base64
+
+import scala.collection.mutable
+
+import org.json4s._
+import org.json4s.JsonAST.JValue
+import org.json4s.jackson.JsonMethods.{compact, pretty, render}
+
+import org.apache.spark.annotation.Unstable
+import org.apache.spark.sql.Row
+import org.apache.spark.sql.catalyst.CatalystTypeConverters
+import org.apache.spark.sql.catalyst.util.{DateFormatter, DateTimeUtils, TimestampFormatter}
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types._
+import org.apache.spark.unsafe.types.CalendarInterval
+
+
+object ToJsonUtil {
+ /**
+ * The compact JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def json(row: Row): String = compact(jsonValue(row))
+
+ /**
+ * The pretty (i.e. indented) JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def prettyJson(row: Row): String = pretty(render(jsonValue(row)))
+
+ /**
+ * JSON representation of the row.
+ *
+ * Note that this only supports the data types that are also supported by
+ * [[org.apache.spark.sql.catalyst.encoders.RowEncoder]].
+ *
+ * @return the JSON representation of the row.
+ */
+ private[sql] def jsonValue(row: Row): JValue = {
+ require(row.schema != null, "JSON serialization requires a non-null schema.")
+
+ lazy val zoneId = DateTimeUtils.getZoneId(SQLConf.get.sessionLocalTimeZone)
+ lazy val dateFormatter = DateFormatter()
+ lazy val timestampFormatter = TimestampFormatter(zoneId)
+
+ // Convert an iterator of values to a json array
+ def iteratorToJsonArray(iterator: Iterator[_], elementType: DataType): JArray = {
+ JArray(iterator.map(toJson(_, elementType)).toList)
+ }
+
+ // Convert a value to json.
+ def toJson(value: Any, dataType: DataType): JValue = (value, dataType) match {
Review Comment:
Yeah, it's my bad
--
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] cloud-fan commented on a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1213838714
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/Row.scala:
##########
@@ -526,95 +513,4 @@ trait Row extends Serializable {
private def getAnyValAs[T <: AnyVal](i: Int): T =
if (isNullAt(i)) throw QueryExecutionErrors.valueIsNullError(i)
else getAs[T](i)
-
- /**
- * The compact JSON representation of this row.
- * @since 3.0
- */
- @Unstable
- def json: String = compact(jsonValue)
Review Comment:
although this is an unstable API, can we still add a migration guide and say that people can use `ToJsonUtil` instead?
--
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] LuciferYang commented on a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1213859516
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/util/ToJsonUtil.scala:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.util
+
+import java.sql.{Date, Timestamp}
+import java.time.{Instant, LocalDate}
+import java.util.Base64
+
+import scala.collection.mutable
+
+import org.json4s._
+import org.json4s.JsonAST.JValue
+import org.json4s.jackson.JsonMethods.{compact, pretty, render}
+
+import org.apache.spark.annotation.Unstable
+import org.apache.spark.sql.Row
+import org.apache.spark.sql.catalyst.CatalystTypeConverters
+import org.apache.spark.sql.catalyst.util.{DateFormatter, DateTimeUtils, TimestampFormatter}
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types._
+import org.apache.spark.unsafe.types.CalendarInterval
+
+
+object ToJsonUtil {
+ /**
+ * The compact JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def json(row: Row): String = compact(jsonValue(row))
+
+ /**
+ * The pretty (i.e. indented) JSON representation of this row.
+ * @since 3.5
+ */
+ @Unstable
+ def prettyJson(row: Row): String = pretty(render(jsonValue(row)))
+
+ /**
+ * JSON representation of the row.
+ *
+ * Note that this only supports the data types that are also supported by
+ * [[org.apache.spark.sql.catalyst.encoders.RowEncoder]].
+ *
+ * @return the JSON representation of the row.
+ */
+ private[sql] def jsonValue(row: Row): JValue = {
+ require(row.schema != null, "JSON serialization requires a non-null schema.")
+
+ lazy val zoneId = DateTimeUtils.getZoneId(SQLConf.get.sessionLocalTimeZone)
+ lazy val dateFormatter = DateFormatter()
+ lazy val timestampFormatter = TimestampFormatter(zoneId)
+
+ // Convert an iterator of values to a json array
+ def iteratorToJsonArray(iterator: Iterator[_], elementType: DataType): JArray = {
+ JArray(iterator.map(toJson(_, elementType)).toList)
+ }
+
+ // Convert a value to json.
+ def toJson(value: Any, dataType: DataType): JValue = (value, dataType) match {
Review Comment:
should we make this function to `private[sql]`, `JValue` is a definition in json4s, and I think we should try to avoid exposing it to users in the public api(https://github.com/apache/spark/pull/36885#discussion_r898644147)
--
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] amaliujia commented on a diff in pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on code in PR #41425:
URL: https://github.com/apache/spark/pull/41425#discussion_r1213976170
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/Row.scala:
##########
@@ -526,95 +513,4 @@ trait Row extends Serializable {
private def getAnyValAs[T <: AnyVal](i: Int): T =
if (isNullAt(i)) throw QueryExecutionErrors.valueIsNullError(i)
else getAs[T](i)
-
- /**
- * The compact JSON representation of this row.
- * @since 3.0
- */
- @Unstable
- def json: String = compact(jsonValue)
Review Comment:
added migration guide and also mark both methods as deprecated.
--
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] hvanhovell commented on pull request #41425: [SPARK-43919][SQL] Extract JSON functionality out of Row
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #41425:
URL: https://github.com/apache/spark/pull/41425#issuecomment-1579043028
Merging to master
--
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] amaliujia commented on pull request #41425: [SPARK-43919] Extract JSON functionality out of Row
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on PR #41425:
URL: https://github.com/apache/spark/pull/41425#issuecomment-1572556520
@hvanhovell @cloud-fan
--
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
Re: [PR] [SPARK-43919][SQL] Extract JSON functionality out of Row [spark]
Posted by "tfinn-ias (via GitHub)" <gi...@apache.org>.
tfinn-ias commented on PR #41425:
URL: https://github.com/apache/spark/pull/41425#issuecomment-1893907566
In 3.4.0 the jsonValue() was callable from Java, and the Javadoc did not indicate it was unstable or unavailable.
In 3.4.1 it fails at runtime with a method not found exception, as far as I can tell due to this change. This is despite the 3.4.1 javadocs still listing its presence.
However, after comparing the Spark 3.4.1 changelog (https://spark.apache.org/releases/spark-release-3-4-1.html) to databricks' version of the Spark 3.4.1 changelog (https://docs.databricks.com/en/release-notes/runtime/13.3lts.html#apache-spark) it seems that they've included more patches than upstream? In particular, they seem to have pulled this patch into their distribution of 3.4.1, but not the later patch that puts the method back. So it might actually be databricks mangling the artifact, I'll follow up with them before getting back if they say it's upstream spark.
--
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