Skip to content

[SPARK-56804][SQL] Add bulk read+convert path for DATE to TimestampNTZ Parquet vector updater#55971

Closed
LuciferYang wants to merge 4 commits into
apache:masterfrom
LuciferYang:SPARK-56804-date-to-tsntz
Closed

[SPARK-56804][SQL] Add bulk read+convert path for DATE to TimestampNTZ Parquet vector updater#55971
LuciferYang wants to merge 4 commits into
apache:masterfrom
LuciferYang:SPARK-56804-date-to-tsntz

Conversation

@LuciferYang
Copy link
Copy Markdown
Contributor

@LuciferYang LuciferYang commented May 19, 2026

What changes were proposed in this pull request?

Extend the bulk-read pattern (SPARK-56791 / 56801 / 56802 / 56803) to DateToTimestampNTZUpdater. Add a readIntegersAsTimestampMicros default method on VectorizedValuesReader, override it in VectorizedPlainValuesReader to fetch source bytes once via getBuffer(total * 4) and run a tight conversion loop, and reduce DateToTimestampNTZUpdater.readValues to a one-line delegation. Scope: UTC, CORRECTED rebase mode; the LEGACY / EXCEPTION rebase variants (handled by DateToTimestampNTZWithRebaseUpdater) are out of scope.

Why are the changes needed?

This was first proposed in #55855 and closed because benchmarks showed no measurable gain -- the per-row work was dominated by DateTimeUtils.daysToMicros's LocalDate / ZonedDateTime / Instant allocation chain, swamping the savings from collapsing N getBuffer(4) slice allocations.

SPARK-56874 fixed that by fast-pathing daysToMicros at ZoneOffset.UTC to Math.multiplyExact(days.toLong, MICROS_PER_DAY). With the conversion now cheap, the bulk-read savings become visible:

JDK Master baseline With this PR Speedup
17 2.8 ns/row (357.5 M/s) 1.7 ns/row (605.2 M/s) ~1.7x
21 2.7 ns/row (366.1 M/s) 1.1 ns/row (934.8 M/s) ~2.5x
25 2.6 ns/row (378.3 M/s) 1.1 ns/row (884.9 M/s) ~2.4x

Does this PR introduce any user-facing change?

No.

How was this patch tested?

New unit tests in ParquetVectorUpdaterSuite for readIntegersAsTimestampMicros (various batch sizes, single-value readValue, UTC conversion semantics). New integration test in ParquetIOSuite for INT32 DATE -> TimestampNTZType through the vectorized reader with both dictionary-encoded and plain pages. Benchmark numbers above are from GHA.

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

No

…to-tsntz

# Conflicts:
#	sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetIOSuite.scala
@LuciferYang LuciferYang marked this pull request as draft May 19, 2026 02:37
@LuciferYang LuciferYang marked this pull request as ready for review May 19, 2026 04:49
@LuciferYang
Copy link
Copy Markdown
Contributor Author

cc @yaooqinn @dongjoon-hyun

Copy link
Copy Markdown
Member

@yaooqinn yaooqinn left a comment

Choose a reason for hiding this comment

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

LGTM.

LuciferYang added a commit that referenced this pull request May 19, 2026
…Z Parquet vector updater

### What changes were proposed in this pull request?

Extend the bulk-read pattern (SPARK-56791 / 56801 / 56802 / 56803) to `DateToTimestampNTZUpdater`. Add a `readIntegersAsTimestampMicros` default method on `VectorizedValuesReader`, override it in `VectorizedPlainValuesReader` to fetch source bytes once via `getBuffer(total * 4)` and run a tight conversion loop, and reduce `DateToTimestampNTZUpdater.readValues` to a one-line delegation. Scope: UTC, `CORRECTED` rebase mode; the `LEGACY` / `EXCEPTION` rebase variants (handled by `DateToTimestampNTZWithRebaseUpdater`) are out of scope.

### Why are the changes needed?

This was first proposed in #55855 and closed because benchmarks showed no measurable gain -- the per-row work was dominated by `DateTimeUtils.daysToMicros`'s `LocalDate` / `ZonedDateTime` / `Instant` allocation chain, swamping the savings from collapsing N `getBuffer(4)` slice allocations.

SPARK-56874 fixed that by fast-pathing `daysToMicros` at `ZoneOffset.UTC` to `Math.multiplyExact(days.toLong, MICROS_PER_DAY)`. With the conversion now cheap, the bulk-read savings become visible:

| JDK | Master baseline | With this PR | Speedup |
|---|---|---|---|
| 17 | 2.8 ns/row (357.5 M/s) | 1.7 ns/row (605.2 M/s) | ~1.7x |
| 21 | 2.7 ns/row (366.1 M/s) | 1.1 ns/row (934.8 M/s) | ~2.5x |
| 25 | 2.6 ns/row (378.3 M/s) | 1.1 ns/row (884.9 M/s) | ~2.4x |

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

New unit tests in `ParquetVectorUpdaterSuite` for `readIntegersAsTimestampMicros` (various batch sizes, single-value `readValue`, UTC conversion semantics). New integration test in `ParquetIOSuite` for INT32 DATE -> `TimestampNTZType` through the vectorized reader with both dictionary-encoded and plain pages. Benchmark numbers above are from GHA.

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

No

Closes #55971 from LuciferYang/SPARK-56804-date-to-tsntz.

Authored-by: YangJie <yangjie01@baidu.com>
Signed-off-by: yangjie01 <yangjie01@baidu.com>
(cherry picked from commit 19073d8)
Signed-off-by: yangjie01 <yangjie01@baidu.com>
@LuciferYang
Copy link
Copy Markdown
Contributor Author

Merged into master and branch-4.3. Thanks @yaooqinn

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