This is an automated email from the ASF dual-hosted git repository.
cloud-fan pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new afd63063c7ed [SPARK-58133][SQL] XML schema inference must not skip
empty values to avoid data loss
afd63063c7ed is described below
commit afd63063c7edb82ccd00764515d700e6444a2058
Author: Wenchen Fan <[email protected]>
AuthorDate: Mon Jul 20 22:52:04 2026 +0800
[SPARK-58133][SQL] XML schema inference must not skip empty values to avoid
data loss
### What changes were proposed in this pull request?
In `XmlInferSchema.inferFrom`, stop skipping empty values during schema
inference. Previously the guard was:
```scala
if (value == null || value.isEmpty || value == options.nullValue) {
return typeSoFar
}
```
This PR drops the `value.isEmpty` term:
```scala
if (value == null || value == options.nullValue) {
return typeSoFar
}
```
so an empty value now falls through the type cascade to `StringType`. As a
result, a field mixing empty and numeric values widens to `StringType` instead
of being inferred as the numeric type.
### Why are the changes needed?
XML schema inference was copied from `CSVInferSchema.inferField`, which
does early-return on `field.isEmpty`. But the two datasources have a crucial
difference at parse time:
- CSV defaults `nullValue` to `""`, so an empty field is read as null by
the parser (`UnivocityParser.nullSafeDatum`). Its inference can safely skip
empty values because they never reach a numeric converter.
- XML defaults `nullValue` to `null`, so an empty value is **not** read as
null. If a column mixing empty and numeric values inferred `LongType`,
`StaxXmlParser` would call `convertTo("", LongType)`, which throws
`NumberFormatException`. In `PERMISSIVE` mode (the default), the
malformed-record error recovery can then silently consume sibling XML events,
**dropping records** — a silent data-loss bug.
Concretely, for input
```xml
<ROW><entry Code="001">first</entry><entry Code="002">second</entry></ROW>
<ROW><entry Code="">third</entry><entry Code="003">fourth</entry></ROW>
<ROW><entry Code="004">fifth</entry><entry Code="">sixth</entry></ROW>
```
`entry._Code` was inferred as `LongType` (from the numeric `Code` values,
empty skipped), and reading then dropped records when it hit the empty
`Code=""`. After this change `_Code` infers as `StringType` and all rows are
read.
By letting empty values fall through to `StringType`, inference stays
consistent with how the XML parser actually reads the data.
### Does this PR introduce _any_ user-facing change?
Yes. With the default `nullValue` (unset), a field that mixes empty and
typed (e.g. numeric) values is now inferred as `StringType` instead of the
typed type. This prevents the silent record-dropping described above. Fields
that are entirely typed, or that use a non-default `nullValue` covering the
empty token, are unaffected. Empty *elements* (`<c/>`) are still read as
`NullType` by the parser and are unaffected.
### How was this patch tested?
Added two tests to `XmlInferSchemaSuite`:
- `SPARK-58133: empty values are not skipped during inference (no data
loss)` — asserts that an attribute mixing empty and numeric values infers
`StringType` and that all rows/entries survive (no PERMISSIVE-mode record
dropping).
- `SPARK-58133: empty and numeric element values still infer via NullType`
— documents that empty elements keep inferring `LongType` (via the parser's
`NullType` path), pinning the element/attribute asymmetry.
Existing XML inference suites pass.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Anthropic Claude Opus)
Closes #57265 from cloud-fan/cloud-fan/xml-infer-empty-dataloss.
Authored-by: Wenchen Fan <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit c585f81cb32c32a2002264f89195aa92d080a4e6)
Signed-off-by: Wenchen Fan <[email protected]>
---
.../spark/sql/catalyst/xml/XmlInferSchema.scala | 12 +++++++-
.../xml/XmlInferSchemaTypeCastingSuite.scala | 15 ++++++---
.../datasources/xml/XmlInferSchemaSuite.scala | 36 ++++++++++++++++++++++
3 files changed, 57 insertions(+), 6 deletions(-)
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/xml/XmlInferSchema.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/xml/XmlInferSchema.scala
index 9df0ebcec7ae..d2a59498549a 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/xml/XmlInferSchema.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/xml/XmlInferSchema.scala
@@ -371,7 +371,17 @@ class XmlInferSchema(private val options: XmlOptions,
private val caseSensitive:
// such values as null (see `StaxXmlParser`), so inference must skip them
too; otherwise a
// column that is entirely `nullValue`s (or mixes them with a typed value)
would infer the
// string content of the token instead of ignoring it.
- if (value == null || value.isEmpty || value == options.nullValue) {
+ //
+ // Unlike `CSVInferSchema.inferField`, an empty value is intentionally NOT
skipped here. The
+ // CSV datasource defaults `nullValue` to `""`, so an empty field is
already read as null by
+ // the parser (`UnivocityParser.nullSafeDatum`) and its inference can
safely skip it. XML's
+ // `nullValue` defaults to `null`, so an empty value is NOT read as null:
`StaxXmlParser`
+ // would call `convertTo("", <numericType>)`, which throws
`NumberFormatException`, and in
+ // PERMISSIVE mode the error recovery can silently drop sibling records.
To keep inference
+ // consistent with the parser, an empty value falls through the cascade to
`StringType`, so a
+ // field mixing empty and numeric values widens to `StringType` rather
than to the numeric
+ // type. See SPARK-58133.
+ if (value == null || value == options.nullValue) {
return typeSoFar
}
diff --git
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/xml/XmlInferSchemaTypeCastingSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/xml/XmlInferSchemaTypeCastingSuite.scala
index 94cd261ae185..dcdd52a4031b 100644
---
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/xml/XmlInferSchemaTypeCastingSuite.scala
+++
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/xml/XmlInferSchemaTypeCastingSuite.scala
@@ -37,7 +37,9 @@ class XmlInferSchemaTypeCastingSuite extends SparkFunSuite
with SQLHelper {
test("String field types are inferred correctly from null types") {
val inferSchema = newInferSchema(Map("timestampFormat" -> "yyyy-MM-dd
HH:mm:ss"))
- assert(inferSchema.inferFrom("", NullType) == NullType)
+ // An empty value carries no numeric/temporal content, so it falls through
the cascade to
+ // StringType (see `inferFrom`); only a genuinely null value preserves
`typeSoFar`.
+ assert(inferSchema.inferFrom("", NullType) == StringType)
assert(inferSchema.inferFrom(null, NullType) == NullType)
assert(inferSchema.inferFrom("100000000000", NullType) == LongType)
// XML infers integral values as LongType (there is no IntegerType
narrowing).
@@ -100,14 +102,17 @@ class XmlInferSchemaTypeCastingSuite extends
SparkFunSuite with SQLHelper {
assert(inferSchema.inferFrom("TRUEe", DoubleType) == StringType)
}
- test("Empty and null values keep the type inferred so far") {
+ test("Null values keep the type inferred so far; empty values widen to
String") {
val inferSchema = newInferSchema(Map.empty[String, String])
- // A genuinely empty/null value carries no type information, so
`typeSoFar` is preserved.
- assert(inferSchema.inferFrom("", NullType) == NullType)
- assert(inferSchema.inferFrom("", LongType) == LongType)
+ // A genuinely null value carries no type information, so `typeSoFar` is
preserved.
assert(inferSchema.inferFrom(null, DoubleType) == DoubleType)
assert(inferSchema.inferFrom(null, TimestampType) == TimestampType)
+ // Unlike CSV, XML inference does NOT skip empty values (see `inferFrom`
and SPARK-58133): the
+ // parser does not read "" as null under the default `nullValue`, so an
empty value falls
+ // through the cascade to StringType, widening any prior numeric/temporal
type to String.
+ assert(inferSchema.inferFrom("", NullType) == StringType)
+ assert(inferSchema.inferFrom("", LongType) == StringType)
}
test("A value matching the nullValue option keeps the type inferred so far")
{
diff --git
a/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/xml/XmlInferSchemaSuite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/xml/XmlInferSchemaSuite.scala
index e1579a53b9dd..e2e6809ad29c 100644
---
a/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/xml/XmlInferSchemaSuite.scala
+++
b/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/xml/XmlInferSchemaSuite.scala
@@ -652,6 +652,42 @@ class XmlInferSchemaSuite
checkAnswer(xmlDF, expectedAns)
}
+ test("SPARK-58133: empty values are not skipped during inference (no data
loss)") {
+ // An attribute that mixes empty ("") and numeric values across rows must
infer as StringType,
+ // not LongType. With the default nullValue (null), the parser does NOT
read "" as null, so if
+ // the column inferred LongType, convertTo("", LongType) would throw
NumberFormatException and
+ // PERMISSIVE-mode error recovery could silently drop sibling records.
Inference therefore lets
+ // an empty value fall through the cascade to StringType, keeping it
consistent with the parser.
+ val rows = Seq(
+ """<ROW><entry Code="001">first</entry><entry
Code="002">second</entry></ROW>""",
+ """<ROW><entry Code="">third</entry><entry
Code="003">fourth</entry></ROW>""",
+ """<ROW><entry Code="004">fifth</entry><entry
Code="">sixth</entry></ROW>""")
+ val df = readData(rows)
+ val entryStruct = df.schema("entry").dataType match {
+ case ArrayType(s: StructType, _) => s
+ case s: StructType => s
+ case other => fail(s"Expected ArrayType(StructType) or StructType for
`entry`, got $other")
+ }
+ assert(entryStruct("_Code").dataType === StringType)
+ // All three rows (six entries) must survive -- the empty attribute must
not trigger the
+ // PERMISSIVE error-recovery path that drops sibling records.
+ assert(df.count() === 3)
+ assert(df.selectExpr("sum(size(entry))").head().getLong(0) === 6)
+ }
+
+ test("SPARK-58133: empty and numeric element values still infer via
NullType") {
+ // Empty *elements* (<c/>) are read as NullType by the parser's EndElement
branch, so a column
+ // mixing empty and numeric elements infers LongType and reads the empty
element as null. This
+ // documents the element/attribute asymmetry (only attribute empties fall
through to String).
+ val rows = Seq(
+ """<ROW><c>1</c></ROW>""",
+ """<ROW><c/></ROW>""",
+ """<ROW><c>3</c></ROW>""")
+ val df = readData(rows)
+ assert(df.schema("c").dataType === LongType)
+ checkAnswer(df, Seq(Row(1L), Row(null), Row(3L)))
+ }
+
test("TIME type inference") {
val xmlString = Seq("""<ROW><t>13:31:24.123456</t></ROW>""")
val df = readData(xmlString)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
