Skip to content

Reset CASE expression caches at input block boundaries - #18889

Draft
ColinLeeo wants to merge 2 commits into
apache:masterfrom
ColinLeeo:fix/case-cache-block-boundary
Draft

ColinLeeo wants to merge 2 commits into
apache:masterfrom
ColinLeeo:fix/case-cache-block-boundary

Conversation

@ColinLeeo

@ColinLeeo ColinLeeo commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

A CASE can leave shared transformer columns cached after one TsBlock or device batch is consumed. Reusing the DAG for the next input can then return stale values or use a column with the wrong length. This change resets the collected nodes before each input and rebuilds missing schema UPDATE common expressions only for devices that passed the filter. Re-evaluating a DIFF within the same block starts from that block's saved incoming state, preserving the previous block's contribution.

This is a narrow extraction from #18826. It keeps the existing selection evaluation and evaluation-time cache clearing mechanism.

Changes

  • Collect each phase's DAG once, deduplicating nodes by identity; reset each node's own cache at the start of a new input.
  • Cover FilterAndProjectOperator, DevicePredicateHandler, and DeviceAttributeUpdater, preserving the original schema constructor descriptors.
  • Apply the final filter mask when an UPDATE common expression must be recomputed, including guarded CAST inputs such as 'n/a'. Query common-expression evaluation retains its existing behavior.
  • Read the selected MultiColumnTransformer input length from the retained column, because consuming the cache can release its stored length.
  • Snapshot the incoming lastValue and null flag for both unary and binary DIFF at each input boundary. Restore that state before transforming the same block again, so CASE cache clearing cannot make a repeated evaluation start from the block's final value. This resolves the shared DIFF failure identified in the review of Fix CASE expression caches leaking across TsBlocks #18826: the second block's common projection returns 4 rather than -1 for inputs [1, 3, 6] followed by [10, 11].
  • Preserve master's DIFF argument evaluation behavior: unary DIFF still requests all rows, while binary DIFF passes the supplied selection to both arguments. In particular, an outer CASE still protects invalid CAST input for binary diff(value, dynamic_boolean). Each DIFF stores only a double and a null flag for the incoming state.
  • Add cross-block/batch, aggregation, effective upper(src) common-expression, guarded UPDATE, nested DIFF, dynamic NULL flags, and outer CASE/CAST compatibility tests. The pending-sibling IT guards existing within-block behavior; it is not the original bug reproduction.

The broader selection-cache redesign from #18826 remains deferred.

Validation

  • 33 calc unit tests and 10 DataNode unit tests passed, including 15 DIFF lifecycle tests and the four existing BlobLength Mockito tests.
  • Ten table-model integration test methods passed: six query methods and four schema methods, including the original three aggregation query variants, the pending-sibling guard, shared unary/binary DIFF across blocks, and binary CASE/CAST protection. The unary literal-flag CAST behavior is also checked against master.
  • A deterministic comparison of 1,200 input blocks matched master's single-evaluation results for unary/binary DIFF, NULL policies, dynamic flags, and selections. Repeating each block on this branch produced the same result; the master implementation reproduced the repeated-evaluation mismatch.
  • Separate baseline/new runs of six schema and six operator probes: eleven failures on master 673434b, all twelve passing on the boundary-reset extraction. The upper(src) test reproduces the null common Column on master.
  • Full reactor test compilation passed in both English and Chinese (53 modules each).
  • Spotless and reactor license checks passed.

Performance

The new-input path clears a precomputed flat array in O(V) per evaluation phase, without a per-block graph walk or visited-set allocation. Schema UPDATE allocates a boolean selection mask and scans its rows only when common expressions exist. The original CASE/common recomputation cost remains.

The following exploratory JMH measurements were taken for the initial boundary-reset extraction, before the DIFF state checkpoint was added. These benchmarks contain no DIFF expressions and were not rerun for this commit. Comparison against master 673434b (JDK 17, 512 MiB/G1, 3 x 1 s warmup, 4 x 1 s measurement, two runs per version in baseline/new/new/baseline order):

1000 rows per block master initial boundary-reset extraction
Addition projection 3.536 us 3.503 us
16 projections sharing an expression 40.932 us 41.549 us
CASE with a common expression 22.749 us 26.916 us

All ten benchmark shapes (five scenarios x 1/1000 rows) validated every result across 16 blocks before measurement. Timings were noisy on the local machine, especially the wide filter scenario; these numbers are not evidence of a general speedup or absence of regressions.

The CASE/common allocation increase was stable: +192 bytes per one-row block and +9176 bytes per 1000-row block. A deterministic counting probe explains the extra work: shared x + 1 was computed 17 times across 16 blocks on master and 32 times here. Master partly benefits from a leftover cache whose predicate happens to remain true for these benchmark inputs; the reset removes that stale-cache reuse, while CASE still clears the result before common materialization. Expensive expressions can therefore amplify this cost.

Collect phase DAG nodes once and clear each cached column before a new block or device batch. Rebuild missing schema UPDATE common expressions with the filter mask while preserving existing selection evaluation.
@ColinLeeo
ColinLeeo requested a lite review from Copilot October 15, 2026 10:15

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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