Skip to content

[SPARK-56486][SQL] Add XSDToSchema.extendDecimalPrecision to prevent …#57348

Open
vboo123 wants to merge 1 commit into
apache:masterfrom
vboo123:SPARK-56486
Open

[SPARK-56486][SQL] Add XSDToSchema.extendDecimalPrecision to prevent …#57348
vboo123 wants to merge 1 commit into
apache:masterfrom
vboo123:SPARK-56486

Conversation

@vboo123

@vboo123 vboo123 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Add a new opt-in utility method XSDToSchema.extendDecimalPrecision(schema: StructType, maxPrecision: Int = 38): StructType that widens DecimalType precision to accommodate full integer-digit capacity.

For each DecimalType(p, s) in the schema, the method raises precision to min(maxPrecision, p + s) while keeping scale unchanged. It recurses into nested StructType fields and ArrayType elements.

The default inference behavior of XSDToSchema.read() is intentionally unchanged: it still maps XSD totalDigits/fractionDigits to DecimalType(totalDigits, fractionDigits) per the W3C spec. Users opt in by calling extendDecimalPrecision on the returned schema.

Why are the changes needed?

XSDToSchema.read() maps an XSD decimal restriction with totalDigits=t and fractionDigits=f to DecimalType(t, f). This is spec-correct, but Spark's DecimalType scale is fixed, whereas the XSD fractionDigits facet is only a maximum. A value that uses fewer fractional digits and more integer digits overflows precision. For example, totalDigits=20, fractionDigits=5 produces DecimalType(20, 5) which allows only 15 integer digits. Values exceeding that are silently read as null in non-ANSI mode, causing data loss with no warning.

This matches the fix proposed by the JIRA reporter: an opt-in utility rather than changing default behavior.

JIRA: https://issues.apache.org/jira/browse/SPARK-56486

Does this PR introduce any user-facing change?

Yes. A new public utility method XSDToSchema.extendDecimalPrecision is added. Existing behavior of XSDToSchema.read() is unchanged.

How was this patch tested?

Three new unit tests in XSDToSchemaSuite:

  • Verifies decimal widening and capping at max precision (e.g. (12,6) -> (18,6), (38,18) unchanged)
  • Verifies recursion into nested struct fields and array elements
  • Verifies an explicit maxPrecision parameter is respected

Ran the suite locally (all pass). Full lint/test matrix via GitHub Actions.

Was this patch authored or co-authored using generative AI tooling?

Generated using Kiro (Claude Opus 4.8)

…silent decimal truncation

XSDToSchema.read maps an XSD decimal restriction (totalDigits=t, fractionDigits=f) to DecimalType(t, f), which is correct per the W3C XML Schema spec. Spark's decimal scale is fixed, however, while the XSD fractionDigits facet is only a maximum, so values that use fewer fractional digits and more integer digits overflow the precision and are silently read as null.

Add an opt-in utility, XSDToSchema.extendDecimalPrecision(schema, maxPrecision), that raises each DecimalType(p, s) to DecimalType(min(maxPrecision, p + s), s), recursing into nested struct fields and array elements. read()'s default inference is unchanged, so existing callers are unaffected.

Generated using Kiro (Claude Opus 4.8)

@shrirangmhalgi shrirangmhalgi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The formula is correct for the stated problem (XSD fractionDigits is a maximum, not fixed → need totalDigits + fractionDigits precision to hold full integer range). Implementation cleanly recurses into nested types; MapType is correctly skipped since XSDToSchema.read() never produces it.

One minor nit: the Scaladoc states maxPrecision "must not exceed DecimalType.MAX_PRECISION" but there's no explicit require - invalid inputs propagate to DecimalType's constructor which throws a less informative error. Consider adding a guard:

require(maxPrecision > 0 && maxPrecision <= DecimalType.MAX_PRECISION,
  s"maxPrecision must be in [1, ${DecimalType.MAX_PRECISION}], got $maxPrecision")

@HyukjinKwon

Copy link
Copy Markdown
Member

Let's make the PR title complete. Seems truncated

@HyukjinKwon

Copy link
Copy Markdown
Member

@sandip-db FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants