Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ private[sql] class JSONOptions(
val columnNameOfCorruptRecord =
parameters.getOrElse("columnNameOfCorruptRecord", defaultColumnNameOfCorruptRecord)

// Whether to ignore column of all null values or empty array/struct during schema inference
val dropFieldIfAllNull = parameters.get("dropFieldIfAllNull").map(_.toBoolean).getOrElse(false)

val timeZone: TimeZone = DateTimeUtils.getTimeZone(
parameters.getOrElse(DateTimeUtils.TIMEZONE_OPTION, defaultTimeZoneId))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class JacksonParser(
val array = convertArray(parser, elementConverter)
// Here, as we support reading top level JSON arrays and take every element
// in such an array as a row, this case is possible.
if (array.numElements() == 0) {
if (array == null || array.numElements() == 0) {
Nil
} else {
array.toArray[InternalRow](schema).toSeq
Expand Down Expand Up @@ -329,8 +329,12 @@ class JacksonParser(
while (nextUntil(parser, JsonToken.END_ARRAY)) {
values += fieldConverter.apply(parser)
}

new GenericArrayData(values.toArray)
// Canonicalize arrays; an array is null if all its elements are null
if (options.dropFieldIfAllNull && values.forall(_ == null)) {
null
} else {
new GenericArrayData(values.toArray)
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,8 @@ class DataFrameReader private[sql](sparkSession: SparkSession) extends Logging {
* that should be used for parsing.</li>
* <li>`samplingRatio` (default is 1.0): defines fraction of input JSON objects used
* for schema inferring.</li>
* <li>`dropFieldIfAllNull` (default `false`): whether to ignore column of all null values or

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How about DataStreamReader? I guess the description should be added to it too.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ok

* empty array/struct during schema inference.</li>
* </ul>
*
* @since 2.0.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ private[sql] object JsonInferSchema {
}.fold(StructType(Nil))(
compatibleRootType(columnNameOfCorruptRecord, parseMode))

canonicalizeType(rootType) match {
canonicalizeType(rootType, configOptions) match {
case Some(st: StructType) => st
case _ =>
// canonicalizeType erases all empty structs, including the only one we want to keep
Expand Down Expand Up @@ -126,9 +126,13 @@ private[sql] object JsonInferSchema {
nullable = true)
}
val fields: Array[StructField] = builder.result()
// Note: other code relies on this sorting for correctness, so don't remove it!
java.util.Arrays.sort(fields, structFieldComparator)
StructType(fields)
if (configOptions.dropFieldIfAllNull && fields.isEmpty) {
NullType
} else {
// Note: other code relies on this sorting for correctness, so don't remove it!
java.util.Arrays.sort(fields, structFieldComparator)
StructType(fields)
}

case START_ARRAY =>
// If this JSON array is empty, we use NullType as a placeholder.
Expand All @@ -140,7 +144,11 @@ private[sql] object JsonInferSchema {
elementType, inferField(parser, configOptions))
}

ArrayType(elementType)
if (configOptions.dropFieldIfAllNull && elementType == NullType) {
NullType
} else {
ArrayType(elementType)
}

case (VALUE_NUMBER_INT | VALUE_NUMBER_FLOAT) if configOptions.primitivesAsString => StringType

Expand Down Expand Up @@ -178,10 +186,10 @@ private[sql] object JsonInferSchema {
/**
* Convert NullType to StringType and remove StructTypes with no fields
*/
private def canonicalizeType(tpe: DataType): Option[DataType] = tpe match {
private def canonicalizeType(tpe: DataType, options: JSONOptions): Option[DataType] = tpe match {
case at @ ArrayType(elementType, _) =>
for {
canonicalType <- canonicalizeType(elementType)
canonicalType <- canonicalizeType(elementType, options)
} yield {
at.copy(canonicalType)
}
Expand All @@ -190,7 +198,7 @@ private[sql] object JsonInferSchema {
val canonicalFields: Array[StructField] = for {
field <- fields
if field.name.length > 0
canonicalType <- canonicalizeType(field.dataType)
canonicalType <- canonicalizeType(field.dataType, options)
} yield {
field.copy(dataType = canonicalType)
}
Expand All @@ -202,7 +210,7 @@ private[sql] object JsonInferSchema {
None
}

case NullType => Some(StringType)
case NullType if !options.dropFieldIfAllNull => Some(StringType)
case other => Some(other)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2408,4 +2408,24 @@ class JsonSuite extends QueryTest with SharedSQLContext with TestJsonData {
spark.read.option("mode", "PERMISSIVE").option("encoding", "UTF-8").json(Seq(badJson).toDS()),
Row(badJson))
}

test("SPARK-23772 ignore column of all null values or empty array during schema inference") {
withTempPath { tempDir =>
val path = tempDir.getAbsolutePath
Seq(
"""{"a":null, "b":[null, null], "c":null, "d":[[], [null]], "e":{}}""",
"""{"a":null, "b":[null], "c":[], "d": [null, []], "e":{}}""",
"""{"a":null, "b":[], "c":[], "d": null, "e":null}""")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you add a test when dropFieldIfAllNull is set to true but not all values in a column are nulls

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ok

.toDS().write.mode("overwrite").text(path)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you need the overwrite mode?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ok

val df = spark.read.format("json")
.option("dropFieldIfAllNull", true)
.load(path)
val expectedSchema = new StructType()
.add("a", NullType).add("b", NullType).add("c", NullType).add("d", NullType)
.add("e", NullType)
assert(df.schema === expectedSchema)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems the DefaultEquality is used here which applies ==. Are there any reasons for === instead of just ==?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No, there's no explicit preference between them since the preferences are diverted even in committers. It's fine to use one of them.

val nullRow = Row(null, null, null, null, null)
checkAnswer(df, nullRow :: nullRow :: nullRow :: Nil)
}
}
}