You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Fix CASE expression caches leaking across TsBlocks - #18826
A CASE branch used to publish a partial, masked column into the same reference-counted cache as a full result. CASE then invalidated its descendants to prevent that partial result being reused. This could discard a pending sibling's column, recompute a shared expression with a fresh reference count, and leave stale results for the next TsBlock. Re-evaluating a stateful diff() also advanced its state twice and silently returned incorrect values.
Separate selected evaluation from full-column caching. getColumnWithSelection() returns a caller-owned column directly, borrowing a full cached column when available without consuming its reference. Composite transformers retain these returned columns; partial results never enter the shared full cache. CASE now reuses its first WHEN through tryEvaluate() and does not clear any descendants during evaluation. The legacy selection/getColumn API remains available through a separate selected-result slot, so it cannot overwrite a pending full result.
Keep the input boundary explicit in query filtering/projection and device predicate/attribute processing. Each phase collects an identity-deduplicated flat node array once and clears each node before initializing the new input. This releases full results with references left by selected consumers. Child edges are declared once; input reset is final, while explicit clearCache() stays overridable for existing Mockito fixtures. After filter materialization/compaction and after projection output construction, a flat cleanup pass releases ordinary caches in finally. The output TsBlock keeps its column references; DIFF's per-input results and cross-block history are preserved. Existing protected schema constructor signatures are retained as delegating overloads.
diff() retains one complete result for the current input, independently of ordinary cache reference counts. This covers both unary DIFF (diff(x) and a literal ignoreNull argument) and binary DIFF (diff(x, boolean_expression)). Both evaluate complete inputs; binary DIFF also reads every row's ignoreNull flag, including rows outside a CASE/filter mask. All masks and full consumers reuse that result, even after ordinary cache consumption or clearCache(). Only explicit new-input reset releases it while preserving the preceding row's value and NULL state. The lifecycle distinction is documented and tested. The formerly ignored correct-result regression is enabled.
Common columns are materialized with the final WHERE mask in both query and attribute UPDATE paths, preserving original row indexes until compaction. The operator-owned public static append helper has been removed. Guarded casts still skip rejected values such as n/a. Full columns calculated by CASE's first WHEN remain reusable during materialization; projection reads the materialized column through an Identity transformer.
The pre-existing tree-model AND wrong-column defect is tracked independently in #18864 and fixed by PR #18875, a standalone one-line production fix with unit and SQL regression tests against master. This PR currently retains the equivalent correct result while adapting the selection interface; once the independent fix lands, synchronizing with master will remove the duplicated correction/test diff. Mappable UDFs receive only selected rows and their results are expanded back to original indexes; scalar UDFs skip rows where selection is false. Retaining sibling columns and reading Multi's position count from retained columns remain covered.
Validation
Dynamic DIFF and retained-memory follow-up
All 110 calc-commons unit tests pass, including three binary DIFF regressions and five cache/memory regressions. Coverage includes partial/all-false masks, cache exhaustion, ordinary cache clearing within one input, dynamic true/false/NULL flags, state carried across inputs, phase-end release of borrowed ordinary columns, and large DIFF result columns.
All 11 selected PR SQL regressions pass against rebuilt 1C1D nodes: six table CASE/DIFF methods, four device/schema methods, and one tree CASE method. All 45 selected DataNode unit tests also pass (35 operator-memory tests, six device tests, and four existing BlobLength/Mockito tests).
Before the production fixes, the three binary DIFF unit tests and two memory tests fail at 11f1a3fcaf. The real SQL regression is reachable and also fails there: one CASE output is NULL instead of 7.0. These are local reproductions, not inferred paths.
The three phase-cleanup/large-column regressions fail on 4620d0f223 and pass after this follow-up. A real operator probe confirms that ordinary shared caches drop from three retained rows to zero. With 20,000-row/1 MiB TsBlocks, eight nested DIFF results total 1,440,576 B; their retained-memory estimate is now 1,440,576 B rather than 524,288 B, and the peak estimate covers them too.
Full-reactor English and Chinese test compilation, Spotless/Checkstyle, and the changed modules' license checks pass.
Previous master synchronization
Merged apache/master at a498d7d2a4 in 11f1a3fcaf. The only textual conflict was in MultiColumnTransformer: upstream a78f5a736c pre-sized its child-column list while this PR changed the selection method to return a caller-owned column. The resolution preserves both changes. Automatically merged collection preallocation and schema blacklist retry accounting were checked as well.
After synchronization, all 102 calc-commons tests, 252 selected DataNode tests (including the two upstream blacklist retry/rollback tests), full-reactor English build/tests, and Chinese test compilation passed. All 10 PR SQL regressions passed against rebuilt 1C1D nodes with both standalone categories explicitly enabled; table connections specify the table SQL dialect.
All 102 calc-commons tests pass, with no skipped DIFF regression.
All 250 selected DataNode tests pass, including the existing Mockito transformer tests, six device tests, and the new scalar UDF selection test.
Full-reactor English build/tests and Chinese test compilation pass. Spotless and git diff --check pass.
Nine selected TableSimpleIT tests pass after rebuilding the dependency reactor: five CASE query methods and four device/schema methods. Coverage includes shared DIFF in WHERE/projection, always-true ELSE preserving the initial NULL, two CASE masks plus a full DIFF output, aggregation reuse, pending siblings, guarded CAST, SHOW/COUNT DEVICES, cross-batch UPDATE, and a real extracted upper(src) common expression.
The new IoTDBCaseWhenThenIT#testNullableAndInLaterCaseBranch passes on SimpleIT (1 ConfigNode + 1 DataNode) with rebuilt reactor dependencies. NULL AND TRUE correctly leaves the CASE branch unmatched.
The exact operator DIFF regression was run with identical dependencies against the 19 pre-fix production classes from aecaa05e84, then the new classes: before expected:<4.0> but was:<-1.0>, after passing. Unit tests additionally cover both ignoreNull settings, all-false masks, cache exhaustion, consecutive blocks, and both output orders.
The earlier source-isolated base/head comparison of the common-expression SQL test remains valid: all 15 original PR-modified classes from merge base fbd5fd22a6 failed with a null common column; the fixed classes passed. This was not a clean full-base reactor build. The pending-sibling test is a guard against destructive cleanup and passes on the original base; it is not the original bug reproducer.
Local IT packaging uses -Drat.skip=true because an unrelated, pre-existing untracked SQL file fails the license scan. New test files include the ASF header.
Cost and lifecycle
DAG collection costs O(V + E) once; each new input uses an O(V) flat reset without a visited-set allocation. A nonempty common list still requires a boolean[n] mask and an O(n) scan of original input rows; an empty list allocates no mask. An additional O(V) flat pass at phase completion releases ordinary caches without graph traversal or visited-set allocation. Filter caches are released before projection starts, and projection caches after the output TsBlock has retained its columns; DIFF's per-input result is not reset by this cleanup. Each DIFF transformer retains its complete result column until the next input, without copying it. Filter/project memory estimates reserve at least one page per distinct DIFF node across both phases, in addition to the existing temporary-column estimate. For larger configured row limits, the bound uses a full DoubleColumn's double array, NULL-flag array, and object/array headers instead of assuming that one page is sufficient. This conservative bound counts shared nodes once and covers nested DIFF chains and sums of independent DIFFs. Selected mappable UDF evaluation additionally builds an int[n] position vector and compacts selected inputs before expanding its output.
The counting regression now verifies one full common-expression evaluation per block and zero masked recomputations for both an ordinary predicate and CASE's first WHEN; projection performs zero expensive expression evaluations. Pure scalar expressions first encountered under distinct branch masks may still be evaluated separately for those masks. Their partial columns are owned by the callers, and stateful DIFF is reused across those requests.
Performance comparison
The measurements below were taken at 2fb7ad284e; they predate master synchronization and the later binary DIFF, phase-cleanup, and memory-estimation follow-ups and were not rerun for those changes.
JMH 1.37 compared the previous PR head aecaa05e84 with this fix using the actual operator, identical dependencies, Corretto 17.0.13/aarch64, 512 MiB G1, one-row blocks, and eight rotating prebuilt inputs. Each revision/scenario used two independent forks, with order reversed in round two, 3 x 1 s warmup and 4 x 1 s measurement. Setup checked every output value. These are operator microbenchmarks, not end-to-end SQL latency estimates.
Scenario
Before, us/block
After, us/block
Before, B/block
After, B/block
x + 1
0.070
0.071
328
328
16 outputs sharing x + 1
0.569
0.576
3640
3640
Filter and project 16 columns
0.856
0.864
13888
13888
Common expression, ordinary predicate
0.289
0.299
2880
2880
Common expression in CASE first WHEN
0.389
0.347
4264
3736
In those measurements, normal-path allocation was unchanged. Timing varies across forks and the non-CASE ranges overlap; no statistically significant speedup/regression is claimed for them. CASE common-expression allocation drops by 528 B/block because its full result is retained instead of recomputed with a mask. The table reports the measured timings without extrapolating to large blocks or expensive UDFs.
This PR has:
been self-reviewed.
added comments explaining the cache lifecycle and intent.
There is a performance concern with the unconditional per-block cache traversal in FilterAndProjectOperator (filter phase, projection phase). Each phase allocates a new identity-based visited set and traverses its transformer DAG. This also adds work to operators with no CASE expressions; an operator performing both filtering and projection does this twice per input block.
I compared the actual operator and changed transformer classes from the merge base fbd5fd22a6c9009dccd7f1a5b84bf7d61e5718e0 against PR head 50c38c16ddf29032a6d1cf1c89b2bcf67627e957, using the same dependencies. The following scenarios contain no CASE expressions. Results below are means across two JVM forks, with one row per input TsBlock:
Operator scenario
Before (µs/block)
PR (µs/block)
Time increase
Additional allocation/block
One addition projection, x + 1
0.084
0.128
~52%
400 B
16 projections sharing s = x + 1, with outputs s + 2 through s + 17
0.762
1.457
~91%
1,440 B
Filter (x0 + 1) > 0, then pass through 16 columns
1.001
1.272
~27%
~772 B
All three scenarios were slower in both forks. Method: JMH 1.37, Zulu JDK 17.0.4.1, one benchmark thread, average-time mode, 3 × 1 s warmup and 4 × 1 s measurement per fork, two forks per revision/scenario with revision order reversed in the second round, a 512 MiB G1 heap, and the GC allocation profiler. The transformer graphs were constructed directly and consumed eight rotating, prebuilt input blocks; setup checked every output against expected values before timing, and each measured invocation returned its result to JMH.
These are operator microbenchmark results, not end-to-end SQL regression percentages. They do not include SQL planning, storage scanning, aggregation, or network costs. The 1024-row variants and the separate 16-independent-additions scenario had substantial variation between forks; a longer-warmup check also showed variation, so I am not drawing a stable, general query-latency regression percentage from those results.
Suggested optimization: once the transformer DAG is fully constructed, collect and identity-deduplicate the nodes once, and retain a flat list for each evaluation phase. The filter-phase list should include the filter and common-expression roots; the projection-phase list should include all projection roots. Both should include CASE descendants. At each new-input boundary, iterate the corresponding list and clear only each node's own cache before initializing the leaves. Keep the existing evaluation-time CASE cleanup semantics unchanged; calling the recursive legacy clearCache() on every list entry would defeat the purpose and could change the CASE boundary behavior.
This should preserve the cross-block correctness fix while removing repeated DAG traversal, identity-set allocation/resizing, and hash lookups from the per-block path. The same approach can be applied to the schema-batch handlers. I recommend addressing this overhead, particularly for workloads producing many small TsBlocks.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick turnaround on the per-block traversal. The precomputed flat lists in 5e0cb1c address my earlier comment, and the original cross-block repro from #18792 now returns the correct rows.
Requesting changes for two problems in this PR:
Regression in the schema UPDATE path: an UPDATE whose CASE guards a common expression now fails where base succeeds (inline on DeviceAttributeUpdater).
IoTDBCaseDeviceCacheIT#testAttributeCommonExpressionClearedByCase does not exercise the path it targets: it passes on base (inline).
Same mechanism, not fixed by this PR. Please fix them here or open follow-ups (details inline):
Within a single block, CASE clears columns that a sibling operand has already computed but not read yet. Results: silent NULLs, NPEs, and UPDATE … SET a = concat(src, CASE …) overwriting attributes with NULL.
MultiColumnTransformer.evaluateWithSelection reads the position count after getColumn(), so CASE … THEN coalesce(x, …) END returns NULL.
diff() under a CASE filter is evaluated twice per block. With this PR the multi-block case returns wrong values instead of failing.
Root cause / design suggestion: short-circuit (evaluateWithSelection) results are stored in the shared, ref-counted ColumnCache, so CASE has to clear caches during evaluation. That clearing destroys columns other consumers still need and forces recomputation within the block (3 and 5; combined with the unmasked recompute, also 1). Resetting at block boundaries cannot help inside a block. Keeping selection-mode results out of the shared cache (for example, returning them directly to the CASE), evaluating the first WHEN with tryEvaluate(), and dropping the evaluation-time clearCache() calls would remove that recompute path and leave the block-boundary reset as a safety net. Item 1 would still need the filter mask.
Out of scope (pre-existing, file not touched by this PR): tree-model LogicAndColumnTransformer.evaluateWithSelection caches rightColumn instead of builder.build() (L80). As a result, CASE WHEN s1 > 100 THEN 1 WHEN s1 > 0 AND s2 > 0 THEN 2 ELSE 3 END returns 2 instead of 3 when s1 is NULL and s2 = 1 (same on base). Probably worth a separate issue.
All SQL in this review was run on 1C1D instances built from this PR (50c38c1, key cases re-checked on 5e0cb1c) and from the merge base fbd5fd2.
The reason will be displayed to describe this comment to others. Learn more.
Regression (must fix):tryEvaluate() recomputes a cleared common expression for every device in the batch, ignoring the filter. If the CASE was guarding that expression, the UPDATE now evaluates it on devices the guard excluded:
CREATETABLEt (id STRING TAG, src STRING ATTRIBUTE, a STRING ATTRIBUTE, v DOUBLE FIELD);
INSERT INTO t(time, id, src, a, v) VALUES (1, 'd0', 'n/a', 'seed', 1), (2, 'd1', '5', 'seed', 1);
UPDATE t SET a = CAST(CAST(src AS INT32) +1AS STRING)
WHERE CASE WHEN src <>'n/a' THEN CAST(src AS INT32) >10 ELSE false END;
base: succeeds; no device matches, so a stays seed
this PR: 305: internal error. statemachine throws a runtime exception: org.apache.iotdb.commons.exception.SemanticException: Cannot cast n/a to INT32 type
If d1 has src = '50' instead (one device matches), base fails with the NPE this PR targets and this PR fails with the same cast error. The expected result is a = '51' for d1.
The query path has always behaved this way (FilterAndProjectOperator L191-L199): SELECT CAST(s AS INT32) + 1 FROM z4 WHERE CASE WHEN s <> 'n/a' THEN CAST(s AS INT32) > 10 ELSE false END fails with 701: Cannot cast n/a to INT32 type on both base and this PR.
Suggested fix for both call sites: recompute only the rows that passed the filter, for example
// ColumnTransformerpublicvoidtryEvaluateWithSelection(boolean[] selection) {
if (!columnCache.hasCached()) {
evaluateWithSelection(selection);
}
}
// here (in FilterAndProjectOperator: filterColumn / positionCount)boolean[] selection = newboolean[deviceSchemaBatch.size()];
for (inti = 0; i < selection.length; i++) {
selection[i] = satisfy(curFilterColumn, i);
}
for (ColumnTransformertransformer : commonTransformerList) {
transformer.tryEvaluateWithSelection(selection);
filterResultColumns.add(transformer.getColumn());
}
With this change, both UPDATEs and the SELECT return the expected results in my local run, and the calc-commons tests plus the datanode *ColumnTransformerTest / DevicePredicateHandlerTest still pass.
This block also duplicates FilterAndProjectOperator.getFilterTsBlock. The two copies have already drifted once: base had tryEvaluate() there but not here, which is the NPE this PR fixes. A single helper next to constructFilteredTsBlock would keep the fix in one place.
The reason will be displayed to describe this comment to others. Learn more.
Recomputing here also re-runs stateful functions on the same block after CASE has cleared them. Before this PR the multi-block case below failed; now it silently returns wrong values:
CREATETABLEdt2 (timeTIMESTAMPTIME, dev STRING TAG, x DOUBLE FIELD);
INSERT INTO dt2(time, dev, x) VALUES (1, 'd0', 1.0), (2, 'd0', 3.0), (3, 'd0', 6.0);
FLUSH;
INSERT INTO dt2(time, dev, x) VALUES (4, 'd0', 10.0), (5, 'd0', 11.0);
FLUSH;
SELECTtime, x, diff(x) AS d FROM dt2 WHERE CASE WHEN diff(x) >1.0 THEN 1.0 ELSE 0.0 END >0.0;
(1C1D with max_tsblock_line_number=1, so [1, 3, 6] and [10, 11] reach this operator as separate blocks)
expected: d = 2.0, 3.0, 4.0 at time 2, 3, 4
base: 301: Index 2 out of bounds for length 2
this PR: time 4 returns d = -1.0. The first WHEN already computed diff for [10, 11] and advanced its state to 11. CASE then clears that column, and this line runs diff again on the same block (10 - 11).
With ELSE 1.0 (every row passes), time 1 returns -5.0 instead of NULL.
A selection mask (see the DeviceAttributeUpdater comment) doesn't help here, because DIFF evaluates every row anyway. The column computed by the first WHEN was complete and valid; it is lost only because CASE clears its WHEN subtree (see the root-cause note in the review summary).
The reason will be displayed to describe this comment to others. Learn more.
DIFF now retains one complete result per input block for masked and full consumers. CASE no longer clears it during evaluation; added the shared-DIFF SQL regressions.
The reason will be displayed to describe this comment to others. Learn more.
This preserves a pending sibling column only when the shared node is itself a CASE. CASE's own cleanup (L222-L227) still clears its WHEN/THEN/ELSE subtrees recursively. Any non-CASE node shared with an operand that the parent has already computed but not read yet is therefore wiped within the same block. All of these fail the same way on base and on this PR, with a single input block:
SQL
expected
actual
SELECT s1 + CASE WHEN s1 > 0 THEN 2 ELSE 3 END FROM p (s1 = 1, -2)
3.0, 1.0
NULL, NULL (tree model too)
SELECT time, s1 FROM p WHERE s1 + CASE WHEN s1 > 0 THEN 2 ELSE 3 END > 0
2 rows
empty set
SELECT s1 > CASE WHEN s1 > 0 THEN 2 ELSE -3 END FROM p
false, true
301: … "leftColumn" is null
SHOW DEVICES / COUNT DEVICES … WHERE src = CASE WHEN src = 'match' THEN 'match' ELSE 'other' END
all devices
301: … "leftColumn" is null
UPDATE t SET a = concat(src, CASE WHEN src = 'match' THEN '-m' ELSE '-o' END)
match-m / other-o
succeeds, but every a becomes NULL (old value lost)
greatest(s1, CASE …), s1 BETWEEN -5 AND CASE …, substring(s, 1, CASE …), CASE WHEN s IS NOT NULL THEN substring(s, 1, length(s) - 1) END
BinaryColumnTransformer.evaluateWithSelection() already avoids this by reading the left column before evaluating the right one (see its comment). I applied that rule everywhere: read each child's column and position count right after evaluating it, before evaluating the next sibling. In my local run that fixes every row above, and the calc-commons tests and datanode transformer tests still pass.
The same argument applies to this opt-out itself: "preserve pending sibling columns" holds for every node type, not just CASE. The new tests only use the CASE … = src orientation, which happens to avoid the problem (see the IT comment).
The reason will be displayed to describe this comment to others. Learn more.
Removed CASE's evaluation-time child-cache clearing and kept selected results outside the shared cache. Composite transformers now retain each child column before evaluating the next sibling.
The reason will be displayed to describe this comment to others. Learn more.
Not changed by this PR, but same mechanism, same class: evaluateWithSelection() reads getColumnCachePositionCount() at L72 after getColumn() has already released the first child's column (when that child has a single reference), so the result has 0 rows. Any multi-argument function under THEN/ELSE whose first argument is referenced only once returns NULL, on base and on this PR:
SELECTtime, CASE WHEN time>0 THEN coalesce(s1, 0.0) END FROM p -- expected 1.0, -2.0; actual NULL, NULLSELECTtime, CASE WHEN time>0 THEN greatest(s1, 0.5) END FROM p -- expected 1.0, 0.5; actual NULL, NULL
8bf2294 fixed this exact pattern in LogicAnd / LogicOr / LogicalAndMulti / LogicalOrMulti / MappableUDF but missed this class. Reading the position count before getColumn(), as evaluate() does at L50-L51, fixes it.
The reason will be displayed to describe this comment to others. Learn more.
Nit: CASE now keeps the same child list in three places: here, the clear loop in doTransform() (L222-L227) and close() (L255-L263). Since the goal is a single child-edge definition, consider forEachChild(ColumnTransformer::clearCache) in doTransform() and forEachChild(ColumnTransformer::close) in close(). The same applies to the close() methods of Unary/Binary/Ternary/Multi/MappableUDF. That way a new child edge cannot be added to one list and missed in another.
The reason will be displayed to describe this comment to others. Learn more.
Optional (same as base, not blocking): the reset runs when the next block arrives. Columns left over from the previous block therefore stay reachable while the driver is blocked and after the last block; an example is a shared expression recomputed after CASE, which keeps an unconsumed reference. Resetting at the end of next() (in a finally) would release them as soon as the output block is built.
The reason will be displayed to describe this comment to others. Learn more.
This test does not exercise the common-expression path it is named after. src is a bare column reference: ColumnTransformerBuilder.visitSymbolReference turns it into a leaf without consulting hasSeen, so commonTransformerList stays empty. The same UPDATE already returns the correct result on base (checked with max_tsblock_line_number=1 and one schema region). The unit test at DevicePredicateHandlerTest L167 likewise uses a leaf as the common transformer, which the planner never produces.
A statement that does go through the common-expression path:
UPDATE common_cache SET a =upper(src) WHERE CASE WHEN upper(src) ='MATCH' THEN true ELSE false END
base: 305: internal error. statemachine throws a runtime exception: java.lang.NullPointerException: Cannot invoke "org.apache.tsfile.block.column.Column.isNull(int)" because "curColumn" is null
this PR: d0/d1 get MATCH, d2/d3 stay NULL
Please also cover the guarded-cast UPDATE from the DeviceAttributeUpdater comment.
The reason will be displayed to describe this comment to others. Learn more.
All new schema tests (and DevicePredicateHandlerTest) put the CASE on the left: CASE … = src. The mirrored predicate src = CASE … END already fails in the first batch on both base and this PR (301: … "leftColumn" is null; see the comment on shouldClearChildrenCache()). Please cover both orientations once that is fixed.
The reason will be displayed to describe this comment to others. Learn more.
Nit: this duplicates the production ValuesOperator in the same package. new ValuesOperator(new TestOperatorContext(), Arrays.asList(blocks)) should do the same job, and it won't need updating when the Operator interface changes.
The reason will be displayed to describe this comment to others. Learn more.
filter.input(0) as a common transformer cannot come from the planner (bare column references never become common expressions). This test therefore exercises the recompute mechanism, but not a DAG the planner builds. A shared non-leaf node (for example, a function over input) would match what UPDATE … SET a = upper(src) WHERE CASE WHEN upper(src) = 'MATCH' … produces.
Share masked common-column materialization between query and schema UPDATE paths. Retain sibling columns before later evaluations can clear their caches, and read multi-argument sizes from retained columns.
Track the known DIFF failure with a correct-result reproducer. Guard common-expression evaluation counts, document helper side effects and shallow array accounting, and retain constructor descriptors used by precompiled subclasses.
Keep the caller-owned evaluateSelection result and upstream child-list preallocation in MultiColumnTransformer. Preserve automatically merged CASE/list optimizations and schema blacklist retry accounting.
The reason will be displayed to describe this comment to others. Learn more.
Added per-input full-result retention to binary DiffColumnTransformer, including complete value/ignoreNull inputs and new-input reset. Added dynamic-argument unit and SQL regressions.
The reason will be displayed to describe this comment to others. Learn more.
Both peak and retained-memory estimates now include one page per distinct unary/binary DIFF node, deduplicated across phases. Added nested/independent DIFF and shared-node regressions.
For the per-block traversal comment: query and schema paths now collect identity-deduplicated node arrays once. Each input block clears the flat array without a new DAG traversal or visited-set allocation.
After the projection columns are consumed, update() never releases the ordinary caches in projectCacheTransformers; this reset only runs when the next batch starts. A completed batch therefore keeps its full project columns (and any retained per-input DIFF result) alive while the next device batch is accumulated, and an exception from the update callback keeps them until close. Wrap the result construction/update path in a finally that clears these caches, as FilterAndProjectOperator does.
This issue also appears on line 155 of the same file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Closes #18792.
A CASE branch used to publish a partial, masked column into the same reference-counted cache as a full result. CASE then invalidated its descendants to prevent that partial result being reused. This could discard a pending sibling's column, recompute a shared expression with a fresh reference count, and leave stale results for the next TsBlock. Re-evaluating a stateful
diff()also advanced its state twice and silently returned incorrect values.Separate selected evaluation from full-column caching.
getColumnWithSelection()returns a caller-owned column directly, borrowing a full cached column when available without consuming its reference. Composite transformers retain these returned columns; partial results never enter the shared full cache. CASE now reuses its first WHEN throughtryEvaluate()and does not clear any descendants during evaluation. The legacy selection/getColumn API remains available through a separate selected-result slot, so it cannot overwrite a pending full result.Keep the input boundary explicit in query filtering/projection and device predicate/attribute processing. Each phase collects an identity-deduplicated flat node array once and clears each node before initializing the new input. This releases full results with references left by selected consumers. Child edges are declared once; input reset is final, while explicit
clearCache()stays overridable for existing Mockito fixtures. After filter materialization/compaction and after projection output construction, a flat cleanup pass releases ordinary caches infinally. The output TsBlock keeps its column references; DIFF's per-input results and cross-block history are preserved. Existing protected schema constructor signatures are retained as delegating overloads.diff()retains one complete result for the current input, independently of ordinary cache reference counts. This covers both unary DIFF (diff(x)and a literal ignoreNull argument) and binary DIFF (diff(x, boolean_expression)). Both evaluate complete inputs; binary DIFF also reads every row's ignoreNull flag, including rows outside a CASE/filter mask. All masks and full consumers reuse that result, even after ordinary cache consumption orclearCache(). Only explicit new-input reset releases it while preserving the preceding row's value and NULL state. The lifecycle distinction is documented and tested. The formerly ignored correct-result regression is enabled.Common columns are materialized with the final WHERE mask in both query and attribute UPDATE paths, preserving original row indexes until compaction. The operator-owned public static append helper has been removed. Guarded casts still skip rejected values such as
n/a. Full columns calculated by CASE's first WHEN remain reusable during materialization; projection reads the materialized column through an Identity transformer.The pre-existing tree-model AND wrong-column defect is tracked independently in #18864 and fixed by PR #18875, a standalone one-line production fix with unit and SQL regression tests against master. This PR currently retains the equivalent correct result while adapting the selection interface; once the independent fix lands, synchronizing with master will remove the duplicated correction/test diff. Mappable UDFs receive only selected rows and their results are expanded back to original indexes; scalar UDFs skip rows where selection is false. Retaining sibling columns and reading Multi's position count from retained columns remain covered.
Validation
Dynamic DIFF and retained-memory follow-up
11f1a3fcaf. The real SQL regression is reachable and also fails there: one CASE output is NULL instead of 7.0. These are local reproductions, not inferred paths.4620d0f223and pass after this follow-up. A real operator probe confirms that ordinary shared caches drop from three retained rows to zero. With 20,000-row/1 MiB TsBlocks, eight nested DIFF results total 1,440,576 B; their retained-memory estimate is now 1,440,576 B rather than 524,288 B, and the peak estimate covers them too.Previous master synchronization
Merged
apache/masterata498d7d2a4in11f1a3fcaf. The only textual conflict was inMultiColumnTransformer: upstreama78f5a736cpre-sized its child-column list while this PR changed the selection method to return a caller-owned column. The resolution preserves both changes. Automatically merged collection preallocation and schema blacklist retry accounting were checked as well.After synchronization, all 102 calc-commons tests, 252 selected DataNode tests (including the two upstream blacklist retry/rollback tests), full-reactor English build/tests, and Chinese test compilation passed. All 10 PR SQL regressions passed against rebuilt 1C1D nodes with both standalone categories explicitly enabled; table connections specify the table SQL dialect.
git diff --checkpass.upper(src)common expression.IoTDBCaseWhenThenIT#testNullableAndInLaterCaseBranchpasses on SimpleIT (1 ConfigNode + 1 DataNode) with rebuilt reactor dependencies. NULL AND TRUE correctly leaves the CASE branch unmatched.aecaa05e84, then the new classes: beforeexpected:<4.0> but was:<-1.0>, after passing. Unit tests additionally cover bothignoreNullsettings, all-false masks, cache exhaustion, consecutive blocks, and both output orders.fbd5fd22a6failed with a null common column; the fixed classes passed. This was not a clean full-base reactor build. The pending-sibling test is a guard against destructive cleanup and passes on the original base; it is not the original bug reproducer.Local IT packaging uses
-Drat.skip=truebecause an unrelated, pre-existing untracked SQL file fails the license scan. New test files include the ASF header.Cost and lifecycle
DAG collection costs
O(V + E)once; each new input uses anO(V)flat reset without a visited-set allocation. A nonempty common list still requires aboolean[n]mask and anO(n)scan of original input rows; an empty list allocates no mask. An additionalO(V)flat pass at phase completion releases ordinary caches without graph traversal or visited-set allocation. Filter caches are released before projection starts, and projection caches after the output TsBlock has retained its columns; DIFF's per-input result is not reset by this cleanup. Each DIFF transformer retains its complete result column until the next input, without copying it. Filter/project memory estimates reserve at least one page per distinct DIFF node across both phases, in addition to the existing temporary-column estimate. For larger configured row limits, the bound uses a full DoubleColumn's double array, NULL-flag array, and object/array headers instead of assuming that one page is sufficient. This conservative bound counts shared nodes once and covers nested DIFF chains and sums of independent DIFFs. Selected mappable UDF evaluation additionally builds anint[n]position vector and compacts selected inputs before expanding its output.The counting regression now verifies one full common-expression evaluation per block and zero masked recomputations for both an ordinary predicate and CASE's first WHEN; projection performs zero expensive expression evaluations. Pure scalar expressions first encountered under distinct branch masks may still be evaluated separately for those masks. Their partial columns are owned by the callers, and stateful DIFF is reused across those requests.
Performance comparison
The measurements below were taken at
2fb7ad284e; they predate master synchronization and the later binary DIFF, phase-cleanup, and memory-estimation follow-ups and were not rerun for those changes.JMH 1.37 compared the previous PR head
aecaa05e84with this fix using the actual operator, identical dependencies, Corretto 17.0.13/aarch64, 512 MiB G1, one-row blocks, and eight rotating prebuilt inputs. Each revision/scenario used two independent forks, with order reversed in round two, 3 x 1 s warmup and 4 x 1 s measurement. Setup checked every output value. These are operator microbenchmarks, not end-to-end SQL latency estimates.x + 1x + 1In those measurements, normal-path allocation was unchanged. Timing varies across forks and the non-CASE ranges overlap; no statistically significant speedup/regression is claimed for them. CASE common-expression allocation drops by 528 B/block because its full result is retained instead of recomputed with a mask. The table reports the measured timings without extrapolating to large blocks or expensive UDFs.
This PR has: