Skip to content

[SPARK-59443][SQL] Reduce allocations in ArrowColumnVector decimal reads - #58748

Open
xiaoh1024 wants to merge 1 commit into
apache:masterfrom
xiaoh1024:exp/spark-arrow-decimal
Open

[SPARK-59443][SQL] Reduce allocations in ArrowColumnVector decimal reads#58748
xiaoh1024 wants to merge 1 commit into
apache:masterfrom
xiaoh1024:exp/spark-arrow-decimal

Conversation

@xiaoh1024

Copy link
Copy Markdown

What changes were proposed in this pull request?

Add a small-decimal accessor for Arrow columns with source precision 1-18,
with full-width fallback and existing Spark decimal conversion semantics.
Add regression tests and an Arrow decimal read benchmark.

Why are the changes needed?

See SPARK-59443.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

  • build/sbt catalyst/compile sql/Test/compile
  • build/sbt "sql/testOnly org.apache.spark.sql.vectorized.ArrowColumnVectorSuite"
    (28 tests passed, including 8 new decimal tests).
  • The same suite passed all 28 tests on a big-endian s390x Temurin 17 JVM
    under QEMU, including a native Arrow Decimal128 layout check.
  • Checkstyle for the changed Java file and Scalastyle for both Scala files.
  • Local ArrowDecimalReadBenchmark and allocation comparisons against the
    original accessor.
  • ArrowDecimalReadBenchmark on GitHub Actions
    passed with JDK 17 and Scala 2.13.
  • Fork Build CI passed.

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

AI assistance was used for implementation, tests, and local validation.

Generated-by: Codex CLI 0.154.0

Read native Decimal128 words for source precision 1-18 while preserving full-width fallback and Spark decimal conversion semantics. Add decimal regression coverage and a read benchmark.

@sunchao sunchao left a comment

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.

Reviewed 0710e00. No actionable correctness findings.

Traced accessor selection, Arrow 19's native Decimal128 layout, the signed-long/full-width fallback, and Spark's BigDecimal-backed rounding, overflow, and integral-conversion semantics. Also reviewed the regression tests and benchmark baseline.

Independent validation: a standalone JDK 17 harness using the two accessor classes extracted unchanged from this PR, the PR's compiled Decimal.scala, Arrow 19.0.0, and cached Spark 4.2 supporting jars passed 63,523 old/new accessor comparisons. Coverage included 20,736 randomized rows, values beyond the declared precision and signed-long range, requested precision/scale changes, nulls, buffer reallocation, slicing, and independent returned objects. A separate BigDecimal-constructor parity check also passed.

The fork's build/test checks are successful at this commit. The local harness is focused validation; I did not rerun the full Spark ArrowColumnVectorSuite, the performance benchmark, or native big-endian execution.

@sunchao sunchao left a comment

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.

Approved based on the completed review and focused validation at this unchanged commit.

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.

2 participants