This is an automated email from the ASF dual-hosted git repository.
cloud-fan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 9d760a91baa1 [SPARK-57884][SQL] Make XML schema inference honor
preferDate, consistent with CSV
9d760a91baa1 is described below
commit 9d760a91baa140fcf517a6b754ba75df3eab0f1f
Author: Wenchen Fan <[email protected]>
AuthorDate: Mon Jul 13 01:24:59 2026 +0800
[SPARK-57884][SQL] Make XML schema inference honor preferDate, consistent
with CSV
### What changes were proposed in this pull request?
Make XML schema inference honor the `preferDate` option as a gate on
whether date inference is attempted, matching `CSVInferSchema`.
Currently, `XmlInferSchema.tryParseDouble` and `tryParseTime` fall through
to `tryParseDate(field)` **unconditionally**, ignoring `options.preferDate`. As
a result, with `preferDate=false`, a value that matches the (default) date
format is still inferred as `DateType`. This contradicts the option's purpose —
`preferDate=false` is meant to disable date inference (date/timestamp inference
is ambiguous, and the option lets users opt out) — and diverges from the CSV
datasource, whose `try [...]
This change routes the date attempt through `preferDate`: when `preferDate`
is true, a bare date still infers as `DateType`; when false, date inference is
skipped and the value falls through to timestamp inference. TIME inference is
unaffected (it remains tried ahead of date/timestamp regardless of
`preferDate`, unchanged from before).
### Why are the changes needed?
`preferDate` is documented and structured (see
`XmlOptions.dateFormatInRead`, which is conditioned on `preferDate`) as
controlling whether date inference happens, but the `tryParse*` control flow
does not honor it, so a bare ISO date leaks through as `DateType` regardless.
The CSV datasource — which XML's inference cascade mirrors — already gates date
inference on `preferDate`. This aligns XML with CSV so `preferDate=false`
behaves consistently across the two text datasources.
### Does this PR introduce _any_ user-facing change?
Yes. With `preferDate=false`, XML schema inference no longer infers
`DateType` for date-shaped values; such values now fall through to timestamp
inference (as they already do in CSV). With the default `preferDate=true`,
behavior is unchanged.
### How was this patch tested?
Updated the `preferDate` tests in `XmlInferSchemaSuite` (core) and
`XmlInferSchemaTypeCastingSuite` (catalyst) to assert the gated behavior:
`preferDate=true` -> `DateType`, `preferDate=false` -> `TimestampType`.
Existing XML inference suites pass.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Anthropic Claude Opus)
Closes #56966 from cloud-fan/SPARK-preferDate-xml-followup.
Authored-by: Wenchen Fan <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
---
AGENTS.md | 33 ++++++++++++++----
.../spark/sql/catalyst/xml/XmlInferSchema.scala | 8 +++--
.../xml/XmlInferSchemaTypeCastingSuite.scala | 39 ++++++++++++++++++++--
.../datasources/xml/XmlInferSchemaSuite.scala | 15 +++++----
4 files changed, 76 insertions(+), 19 deletions(-)
diff --git a/AGENTS.md b/AGENTS.md
index 433cff6ccacd..16b9976caba5 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -20,6 +20,15 @@ Spark Connect protocol is defined in proto files under
`sql/connect/common/src/m
Avoid introducing non-ASCII characters in code or comments. String literals
may contain non-ASCII when the content requires it (error messages, test data,
etc.). Identifiers are ASCII by convention. The common failure mode is
typographic characters (em-dash, smart quotes, ellipsis, non-breaking space)
sneaking into comments; scalastyle flags some of these. Spot-check before
committing: `grep -rn -P "[^\x00-\x7F]" <files>`.
+Keep source lines within 100 characters — the linters enforce this for Scala,
Java, and Python, and LLMs commonly overrun it in comments and long
expressions. A quick scan of just the changed files catches most cases in
seconds, far cheaper than a CI round trip:
+
+ { git diff --name-only --diff-filter=ACM HEAD; git ls-files --others
--exclude-standard; } \
+ | grep -E '\.(scala|java|py)$' | sort -u \
+ | xargs -r awk 'length>100 && $0 !~ /^[[:space:]]*(import|package) / &&
$0 !~ /https?:\/\// \
+ {print FILENAME":"FNR": "length" chars"}'
+
+This is only a hint: it approximates the linters' exemptions (imports, URLs)
rather than matching them exactly, so it can over- or under-report. The linters
remain the source of truth.
+
## Scala Test Base Classes
When writing a new Scala test suite, pick the lowest base class that provides
what the test actually needs. Spark uses the `AnyFunSuite` ScalaTest style
throughout, so the bases below are the chain to choose from. Each adds
capability on top of the previous:
@@ -147,22 +156,32 @@ Run a single test case:
## Investigating PR CI Failures
-Do NOT download full job logs to grep for errors — they are very large and
slow. Instead, use the test report annotations on the fork.
+Enumerate all failing check runs first, then drill into each by type. Do not
assume a single failure: a PR can fail tests, linters, and the build at once,
and these surface through different channels.
Step 1 — Get the fork owner and the latest commit SHA of the PR:
gh api repos/apache/spark/pulls/<PR_NUMBER> --jq '{owner:
.head.repo.owner.login, sha: .head.sha}'
-Step 2 — Find the "Report test results" check run on the fork's commit:
+Step 2 — List every failing check run on the fork's commit. This is the
complete failure set:
+
+ gh api repos/<OWNER>/spark/commits/<SHA>/check-runs --paginate \
+ --jq '.check_runs[] | select(.conclusion == "failure") | {name, id: .id}'
+
+A passing (or absent) "Report test results" does NOT mean CI is green. That
check aggregates only test-case failures; linter, license, dependency, MiMa,
compile, and doc-build failures are separate check runs that produce no test
annotations. Always work from the list in Step 2, not from any single check.
+
+Step 3 — Drill into each failure according to its kind:
+
+- **Test jobs** (e.g. "Report test results", "Build modules: ..."): fetch
failure annotations. Each annotation contains the test class, test name, and
failure message:
- gh api repos/<OWNER>/spark/commits/<SHA>/check-runs \
- --jq '.check_runs[] | select(.name == "Report test results") | {id: .id,
annotations: .output.annotations_count}'
+ gh api repos/<OWNER>/spark/check-runs/<CHECK_RUN_ID>/annotations
-Step 3 — Fetch failure annotations:
+- **Non-test jobs** (e.g. "Linters, licenses, and dependencies", "Build"):
find the failed step, then read only that job's log:
- gh api repos/<OWNER>/spark/check-runs/<CHECK_RUN_ID>/annotations
+ gh api repos/<OWNER>/spark/actions/jobs/<JOB_ID> \
+ --jq '{name, steps: [.steps[] | select(.conclusion == "failure") |
.name]}'
+ gh api repos/<OWNER>/spark/actions/jobs/<JOB_ID>/logs
-Each annotation contains the test class, test name, and failure message.
+Avoid downloading the large per-shard *test* job logs — they are very large
and slow; use the annotations for those. Lint, license, dependency, and build
job logs are small and fine to read directly when a step fails.
## Checking PR Merge Status
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 fb534062ec28..b4c942326702 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
@@ -682,16 +682,20 @@ class XmlInferSchema(private val options: XmlOptions,
private val caseSensitive:
} else if (isTimeTypeEnabled && isTime(field)) {
// TIME is tried ahead of date/timestamp, matching the ordering of the
previous cascade.
TimeType(TimeType.DEFAULT_PRECISION)
- } else {
+ } else if (options.preferDate) {
tryParseDate(field)
+ } else {
+ tryParseTimestampNTZ(field)
}
}
private def tryParseTime(field: String): DataType = {
if (isTimeTypeEnabled && isTime(field)) {
TimeType(TimeType.DEFAULT_PRECISION)
- } else {
+ } else if (options.preferDate) {
tryParseDate(field)
+ } else {
+ tryParseTimestampNTZ(field)
}
}
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 c78171d61565..e5570ddde88f 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
@@ -155,11 +155,44 @@ class XmlInferSchemaTypeCastingSuite extends
SparkFunSuite with SQLHelper {
assert(inferSchema.inferFrom("2024-01-15T10:00:00", DateType) ==
TimestampType)
}
- test("date is inferred regardless of preferDate") {
+ test("preferDate gates date inference (consistent with CSV)") {
+ // preferDate controls whether date inference is attempted, matching
CSVInferSchema: when true
+ // a bare date infers as DateType; when false date inference is skipped
and the value falls
+ // through to timestamp inference.
+ assert(newInferSchema(Map("preferDate" -> "true"))
+ .inferFrom("2024-01-15", NullType) == DateType)
+ assert(newInferSchema(Map("preferDate" -> "false"))
+ .inferFrom("2024-01-15", NullType) == TimestampType)
+ }
+
+ test("preferDate gates the incremental temporal re-entry (tryParseTime)") {
+ // Refining a temporal `typeSoFar` re-enters the cascade at
`tryParseTime`, which must honor
+ // `preferDate` just like the fresh path (`tryParseDouble`). With a
`Date`-so-far field seeing
+ // another date-only value: preferDate=true re-infers `Date` and the merge
stays `Date`;
+ // preferDate=false skips date inference so the value falls through to
`Timestamp`, and the
+ // merge widens `Date` to `Timestamp`. This guards the `tryParseTime`
branch, which the
+ // incremental-vs-legacy parity test does not cover (it runs only at the
default
+ // preferDate=true).
+ assert(newInferSchema(Map("preferDate" -> "true"))
+ .inferFrom("2024-01-15", DateType) == DateType)
+ assert(newInferSchema(Map("preferDate" -> "false"))
+ .inferFrom("2024-01-15", DateType) == TimestampType)
+ }
+
+ test("TIME inference precedes the preferDate gate") {
+ // TIME is tried ahead of date/timestamp in both `tryParseDouble` (fresh
path) and
+ // `tryParseTime` (incremental temporal re-entry), so a TIME-shaped value
infers as `TimeType`
+ // regardless of `preferDate`. This guards against a refactor that would
move the TIME guard
+ // inside the `preferDate` branch and silently drop TIME inference when
preferDate=false.
+ val time = TimeType(TimeType.DEFAULT_PRECISION)
Seq("true", "false").foreach { preferDate =>
val inferSchema = newInferSchema(Map("preferDate" -> preferDate))
- assert(inferSchema.inferFrom("2024-01-15", NullType) == DateType,
- s"expected DateType with preferDate=$preferDate")
+ // Fresh path (typeSoFar == NullType, enters via tryParseDouble).
+ assert(inferSchema.inferFrom("10:00:00", NullType) == time,
+ s"expected TimeType with preferDate=$preferDate")
+ // Incremental temporal re-entry (typeSoFar == TimeType, enters via
tryParseTime).
+ assert(inferSchema.inferFrom("10:00:00", time) == time,
+ s"expected TimeType with preferDate=$preferDate")
}
}
}
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 78755d531cd8..2c72825167f0 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
@@ -722,14 +722,15 @@ class XmlInferSchemaSuite
assert(readData(doubleThenLong).schema.fields.head.dataType === DoubleType)
}
- test("date is inferred regardless of preferDate") {
+ test("preferDate gates date inference (consistent with CSV)") {
val xmlDate = Seq("""<ROW><d>2024-01-15</d></ROW>""")
- // preferDate governs which date formatter is used, not whether date
inference is attempted.
- Seq("true", "false").foreach { preferDate =>
- val df = readData(xmlDate, Map("preferDate" -> preferDate))
- assert(df.schema.fields.head.dataType === DateType,
- s"expected DateType with preferDate=$preferDate")
- }
+ // preferDate controls whether date inference is attempted, matching
CSVInferSchema: when true
+ // a bare date infers as DateType; when false date inference is skipped
and the value falls
+ // through to timestamp inference.
+ assert(readData(xmlDate, Map("preferDate" -> "true"))
+ .schema.fields.head.dataType === DateType)
+ assert(readData(xmlDate, Map("preferDate" -> "false"))
+ .schema.fields.head.dataType === TimestampType)
}
test("incremental type casting yields the same schema as the legacy batch
path") {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
