Skip to content

Avoid generated code for constant string-to-timestamp casts during planning - #19514

Open
xiangfu0 wants to merge 1 commit into
apache:masterfrom
xiangfu0:xiangfu0/perf-constant-cast-reduction
Open

Avoid generated code for constant string-to-timestamp casts during planning#19514
xiangfu0 wants to merge 1 commit into
apache:masterfrom
xiangfu0:xiangfu0/perf-constant-cast-reduction

Conversation

@xiangfu0

@xiangfu0 xiangfu0 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Timestamp range predicates such as event_time >= CAST('2024-01-01 00:00:00' AS TIMESTAMP) compile a generated Java reducer for each constant cast during Calcite field trimming. Numeric epoch strings also pay this cost even though Calcite cannot parse them and leaves the casts for later Pinot planning.

Add a narrow executor for ordinary CHAR/VARCHAR literal-to-TIMESTAMP casts. It calls the same Avatica parser and RexBuilder literal construction as Calcite's generated code. Unsupported expression batches still delegate to Calcite; a failed supported batch remains entirely unchanged. This preserves nulls, precision, invalid-input behavior and batch ordering.

Install the executor only during SQL-to-rel conversion, restoring the prior executor before optimization and preserving explicitly supplied executors. Installing it only in a RelBuilder context is insufficient because RelBuilder.transform discards that context.

Benchmark

Independently packaged baseline c6116095f7d3834613649c770d75fed7dd5a874a and this change, with identical benchmark source and bytecode. The benchmark calls QueryEnvironment.compile and closes each compiled query, covering SQL parsing, validation, SQL-to-rel conversion and logical optimization. It cycles through 1,024 prebuilt synthetic account IDs. The measured window query filters a timestamp range and selects the latest event version using ROW_NUMBER().

Warmed results on a shared Apple M4 Pro host with 24 GiB RAM, OpenJDK 25.0.4, JMH 1.37, one benchmark thread, 1 GiB heap and ActiveProcessorCount=4:

Window-query constant form Baseline µs/op PR µs/op Latency change Baseline B/op PR B/op Allocation change
Timestamp-string CAST 1,327.98 483.56 −63.59% 1,332,499 860,004 −35.46%
Epoch-string CAST 1,314.98 506.25 −61.50% 1,335,453 864,977 −35.23%
Typed TIMESTAMP literal control 469.26 478.46 +1.96% 846,269 846,059 −0.02%

Each revision has two independent JVM forks, run baseline/PR/PR/baseline. Each fork uses five 3-second warmups and five 2-second measurements with the GC profiler. Means include every measured iteration. The control's direction reverses between pairs; one baseline control iteration is elevated and retained. These shared-host measurements establish a compilation benefit for these queries, not end-to-end query throughput or precise bounds on small regressions. Shorter pilot runs were excluded because they were still warming up.

Independent fork means and reproduction
Constant form Baseline A/B µs/op PR A/B µs/op
Timestamp-string CAST 1,312.65 / 1,343.30 486.42 / 480.70
Epoch-string CAST 1,329.88 / 1,300.09 496.79 / 515.71
Typed literal control 452.94 / 485.58 486.35 / 470.58

Copy BenchmarkConstantCastPlanning.java to a worktree at the baseline commit. Build each worktree separately with JDK 25:

./mvnw -pl pinot-perf -am package -Ppinot-fastdev -DskipTests \
  -Dskip.npm -Dskip.installnodenpm -Dcheckstyle.skip -Dspotless.check.skip \
  -Dlicense.skip -Denforcer.skip -Dshade.phase.prop=none \
  -Dassembly.skipAssembly=true -Dmaven.javadoc.skip=true

Run the following once in each worktree in baseline/PR/PR/baseline order, without concurrent builds. Use distinct result filenames for each run. Verify the packaged planner and benchmark jars match their module target jars before running.

"$JAVA_HOME/bin/java" -Xmx1g \
  -cp 'pinot-perf/target/pinot-perf-pkg/lib/*' org.openjdk.jmh.Main \
  BenchmarkConstantCastPlanning \
  -p _queryShape=WINDOW \
  -p _literalForm=CAST,EPOCH_STRING_CAST,TIMESTAMP_LITERAL \
  -wi 5 -w 3s -i 5 -r 2s -f 1 -t 1 -foe true -prof gc \
  -jvmArgs '-Xms1g -Xmx1g -XX:ActiveProcessorCount=4' \
  -rf json -rff result.json

The harness also includes a simple FILTER query and a LONG epoch-millis column control for additional experiments.

Separate matched JFR runs corroborate the mechanism: Janino occurs in 142/471 baseline JMH-worker execution samples and 0/750 PR samples. Sampling alone does not establish absence; the integration tests independently count generated reductions exactly.

Validation

  • 900 planner tests passed: 50 executor cases, 8 full-compilation cases, 240 query-compilation cases, 10 type-coercion cases and 592 resource-based query plans.
  • The new full-compilation tests compare plans, row types and table names with typed timestamp literals, assert zero generated reducers, and verify the planner executor is restored. The baseline produces two generated reducers for the same queries while preserving plan equivalence.
  • Differential executor tests cover nulls, fractional precision/rounding, pre-epoch values, whitespace, invalid input, unsupported/mixed batches, custom executors and concurrent calls.
  • Spotless, Checkstyle and license checks pass for both changed modules. Direct JDK 25 lint compilation of all five changed sources has zero added-line warnings.
  • The stricter deprecation-enabled reactor check encounters an existing issue in unchanged ZstandardDecompressor.java:51: Zstd's provided JetBrains annotation dependency is absent from that module's compile classpath. This is reproduced on the identical baseline source; adding the annotation jar resolves that check. No unrelated dependency change is included here.

@codecov-commenter

codecov-commenter commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.66667% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.75%. Comparing base (c611609) to head (881110e).

Files with missing lines Patch % Lines
...org/apache/pinot/calcite/rex/PinotRexExecutor.java 93.54% 1 Missing and 1 partial ⚠️
.../java/org/apache/pinot/query/QueryEnvironment.java 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master   #19514   +/-   ##
=========================================
  Coverage     67.74%   67.75%           
- Complexity     1424     1430    +6     
=========================================
  Files          3489     3490    +1     
  Lines        224672   224708   +36     
  Branches      35468    35475    +7     
=========================================
+ Hits         152210   152252   +42     
+ Misses        60445    60432   -13     
- Partials      12017    12024    +7     
Flag Coverage Δ
integration 100.00% <ø> (+100.00%) ⬆️
integration1 100.00% <ø> (?)
integration2 0.00% <ø> (ø)
java-25 67.75% <91.66%> (+<0.01%) ⬆️
lane-a 100.00% <ø> (+100.00%) ⬆️
lane-b 0.00% <ø> (ø)
temurin 67.75% <91.66%> (+<0.01%) ⬆️
unittests 67.75% <91.66%> (+<0.01%) ⬆️
unittests1 57.81% <91.66%> (+0.01%) ⬆️
unittests2 39.45% <33.33%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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