Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[SPARK-35104][SQL] Fix ugly indentation of multiple JSON records in a single split file generated by JacksonGenerator when pretty option is true #32203

Closed
wants to merge 1 commit into from
Closed

[SPARK-35104][SQL] Fix ugly indentation of multiple JSON records in a single split file generated by JacksonGenerator when pretty option is true #32203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/json/JacksonGenerator.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package org.apache.spark.sql.catalyst.json
import java.io.Writer

import com.fasterxml.jackson.core._
import com.fasterxml.jackson.core.util.DefaultPrettyPrinter

import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.expressions.SpecializedGetters
Expand Down Expand Up @@ -73,7 +74,7 @@ private[sql] class JacksonGenerator(

private val gen = {
val generator = new JsonFactory().createGenerator(writer).setRootValueSeparator(null)
if (options.pretty) generator.useDefaultPrettyPrinter() else generator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useDefaultPrettyPrinter() should return the same default printer, see the method's doc:

    /**
     * Convenience method for enabling pretty-printing using
     * the default pretty printer
     * ({@link com.fasterxml.jackson.core.util.DefaultPrettyPrinter}).
     *
     * @return This generator, to allow call chaining
     */
    public abstract JsonGenerator useDefaultPrettyPrinter();

Could you clarify how your code changes the behavior, please.

Copy link
Member Author

@sarutak sarutak Apr 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course I read the doc but the actually returned instance is not equivalent to the instance created by new DefaultPrettyPrinter("").
https://github.com/FasterXML/jackson-core/blob/jackson-core-2.12.2/src/main/java/com/fasterxml/jackson/core/base/GeneratorBase.java#L452

The parameter of the constructor is the separator which separates multiple root JSON records and the default separator character is " ".
https://github.com/FasterXML/jackson-core/blob/jackson-core-2.12.2/src/main/java/com/fasterxml/jackson/core/util/DefaultPrettyPrinter.java#L101
https://github.com/FasterXML/jackson-core/blob/jackson-core-2.12.2/src/main/java/com/fasterxml/jackson/core/util/DefaultPrettyPrinter.java#L28

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the clarification.

if (options.pretty) generator.setPrettyPrinter(new DefaultPrettyPrinter("")) else generator
}

private val lineSeparator: String = options.lineSeparatorInWrite
Expand Down
25 changes: 25 additions & 0 deletions sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/json/JsonSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2844,6 +2844,31 @@ abstract class JsonSuite
assert(readback.collect sameElements Array(Row(0), Row(1), Row(2)))
}
}

test("SPARK-35104: Fix wrong indentation for multiple JSON even if `pretty` option is true") {
withSQLConf(SQLConf.LEAF_NODE_DEFAULT_PARALLELISM.key -> "1") {
withTempPath { path =>
val basePath = path.getCanonicalPath
val df = Seq("a", "b", "c").toDF
df.write.option("pretty", "true").json(basePath)

val expectedText =
s"""{
| "value" : "a"
|}
|{
| "value" : "b"
|}
|{
| "value" : "c"
|}
|""".stripMargin
val actualText = spark.read.option("wholetext", "true")
.text(basePath).map(_.getString(0)).collect().mkString
assert(actualText === expectedText)
}
}
}
}

class JsonV1Suite extends JsonSuite {
Expand Down