cloud-fan commented on code in PR #58704:
URL: https://github.com/apache/spark/pull/58704#discussion_r4078726161
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -7034,6 +7034,27 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val JSON_STREAM_MULTILINE_TOP_LEVEL_ARRAY =
+ buildConf("spark.sql.json.enableStreamingTopLevelArray")
+ .internal()
Review Comment:
**Non-blocking (P2):** The public JSON option table tells users that
`spark.sql.json.enableStreamingTopLevelArray` is this option's session default,
but `.internal()` removes the key from public configuration discovery. That
gives the same key conflicting support signals. Since the option docs expose it
as user-settable, please make the ConfigEntry public (or stop documenting the
key as a supported default).
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/json/JSONOptions.scala:
##########
@@ -239,6 +239,10 @@ class JSONOptions(
val useUnsafeRow: Boolean =
parameters.get(USE_UNSAFE_ROW).map(_.toBoolean).getOrElse(
SQLConf.get.getConf(SQLConf.JSON_USE_UNSAFE_ROW))
+ val streamMultilineTopLevelArray: Boolean =
+
parameters.get(ENABLE_STREAMING_TOP_LEVEL_ARRAY).map(_.toBoolean).getOrElse(
Review Comment:
**Nit (P3):** Please add the inverse precedence case: session config true
with per-read option false, and assert eager document-scoped malformed
handling. The current test only covers option true over session false, so an
OR-based implementation or one that ignores explicit false would still pass.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -7034,6 +7034,27 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val JSON_STREAM_MULTILINE_TOP_LEVEL_ARRAY =
+ buildConf("spark.sql.json.enableStreamingTopLevelArray")
+ .internal()
+ .doc("When true, multiline JSON reads stream the elements of a top-level
array one at a " +
Review Comment:
**Nit (P3):** `DataFrameReader.json(Dataset[String])` builds the same
`JSONOptions`, so this setting is accepted there, but that path still calls
`rawParser.parse` and materializes the array eagerly. Please scope this help to
multiline file and archive reads, matching the option table, unless the Dataset
path is also wired to `parseIterator`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
