fix: BYTE sum/avg and COUNT ANTI-JOIN CHAIN wrong answers (#9281, #9278, #9277) - #9288

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

Review

Overall this is a small, focused fix and the changes look correct. Notes below.

Type.increment/decrement (#9281)

• Widening Byte to Short before recursing is simple and correct. The result is never a Byte, which matches how the other integral cases behave.
• Minor: Short.valueOf(a.shortValue()) allocates for values outside the cached range on a hot aggregation path. The Byte-first check is cheap, so this is probably fine. A dedicated Byte case in the switch would avoid the extra recursion if you care about the sum() hot loop.
• Tests cover Byte+Byte, Byte+Long, Integer+Byte, Double+Byte and decrement. Nothing covers Byte with Float or BigDecimal, so it would be worth adding those.

Anti-join push-down (#9278)

• Declining the push-down when the relationship has properties, a WHERE, or a variable, or when the end nodes have labels or properties, is the safe fix.
• It also means a negated pattern with a plain label on the end nodes, such as NOT (a:Person)-[:K]->(b:Person), falls back to the row pipeline. That is a performance regression for a plausible common shape. If the label is already implied by the MATCH, you could keep the push-down when the label equals the one bound for that variable. Otherwise, please confirm this trade-off is intended.
• Prefer one pattern per line. final NodePattern first = ..., last = ...; is fine, but the new code already calls pp.getFirstNode() just below, so those calls can reuse first and last.

AntiJoinChainOp (#9277)

• Adding the target-label equality to both reuse conditions is right. Please double-check that nodeLabels has length hops+1, so nodeLabels[h + 1] is always in bounds. It looks that way from the surrounding code.
• The anti-join reuse check compares nodeLabels[antiJoinTargetIdx] to nodeLabels[1]. That assumes hopMaps[0] was built with the label at index 1. The test covers it, but a short comment would help.

Tests

• Good: the "as written" versus "after WITH" cross-check is a strong oracle.
• assertSame shadows the AssertJ naming and reads like an assertion on object identity. Something like assertSameCount would be clearer.
• The test DB path is hard-coded. Existing tests extend TestHelper, which handles creation and cleanup and avoids collisions with parallel builds. Consider using it.
• In Issue9281SumByteTest, isEqualTo(150) on an Object depends on the exact return type (Integer). That is fine, but it is brittle if the type later changes to Long.

No security concerns. Per CLAUDE.md, no new dependencies were added and there are no stray System.out calls.

Up to standards ✅

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

No actionable comments were generated in the recent review. 🎉

Merge Risk: ⚪ Minimal · up to 7d69d

The PR fixes BYTE sum and average, and it fixes wrong anti-join chain counts by declining unsafe push-downs and avoiding map reuse across labels. Regression tests cover these changes, and no outstanding defects were found. It appears ready to merge.

• Fix all pre-merge checks with AI

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

Review of #9288

The fixes look correct and well-scoped. The new tests compare the push-down result against the row pipeline, which is a good oracle.

Suggestions

1. CypherExecutionPlan: info.sourceLabel = first.getLabels().get(0) only looks at the first label. This is safe only if hasPushDownRepresentableLabel rejects multi-label nodes (e.g. (x:A:B)). Please confirm that it does, and add a test such as NOT (x:P:Foo)-[:K]->(z).
2. AntiJoinInfo.sourceLabel and targetLabel are non-final mutable fields set after construction. Passing them through the constructor would let you mark them final, which matches the CLAUDE.md preference.
3. Type.increment and Type.decrement: the Byte pre-check adds an instanceof test and a recursive call on every invocation. The cost is negligible for the Byte path, but this sits on aggregation hot paths. A case Byte branch inside the switch would avoid the extra test for the common types, though this is minor.
4. Byte overflow semantics: byte 100 + byte 50 now returns a Short or Integer (150), which is the desired fix. Please document that sum over BYTE does not wrap. The test asserts isEqualTo(150), so the exact result type is worth pinning down.
5. Tests: Issue9277And9278AntiJoinChainTest uses a hard-coded ./target/databases path. Extending TestHelper would be more consistent, as Issue9281SumByteTest does. Also consider a test for the antiJoinMap = hopMaps[0] reuse path with matching labels, to guard against the optimization regressing into never reusing the map.
6. Style: the all-caps comments (// THE PUSH-DOWN ONLY LOOKS AT...) differ from the surrounding comment style.

No security concerns. Overall this is a good fix, and the points above are minor.

Review

Overall this is a focused fix with good regression tests. No blocking issues found.

Type.increment/decrement (#9281)

• Widening Byte to Short up front is simple and correct. The result is a Short or wider, so 100 + 50 can't overflow.
• One side effect: sum() over BYTE now returns a Short when both operands are bytes. That seems fine, but it's a visible type change. The test asserts isEqualTo(150) on an Object, which works with the SQL path's numeric result. Worth confirming nobody relies on a Byte result.
• Minor perf point: the extra instanceof checks run on every call, including the hot Integer/Long paths. They're cheap, but you could move them to the default branch of the switch to keep the common path untouched.

Anti-join push-down (#9278)

• Declining on edge props, inline WHERE, edge variable, and end-node props/labels is the right conservative fix. It falls back to the row pipeline, which is correct.
• The label check in the chain-matching code (sourceLabel vs nodeLabels[idx]) correctly handles a label that is implied by the chain. Only the first label is compared, and hasPushDownRepresentableLabel presumably rejects multi-label nodes. NOT (x:P:Foo) is covered by a test, which is good.

AntiJoinChainOp (#9277)

• Both reuse conditions now include the target label (nodeLabels[h+1]), which fixes the stale-map bug. The antiJoinMap = hopMaps[0] check uses nodeLabels[antiJoinTargetIdx] vs nodeLabels[1]. That matches the stated reasoning.
• Please double check the non-anchorIsSource branch (and any similar reuse in the OLAP or other execute* paths) for the same pattern. Only executeOLTP was changed here.

Style/CLAUDE.md

• The ALL-CAPS comments (THE PUSH-DOWN ONLY LOOKS AT..., THE MAP KEEPS ONLY...) don't match the surrounding comment style, so use normal case.
• The AntiJoinInfo constructor assigns the new fields before the others. That's harmless but a bit odd.
• Tests use assertThat correctly. The two test classes use different bases (a manual DatabaseFactory vs TestHelper). Using TestHelper in the anti-join test would remove the setup/teardown boilerplate.

Tests

• Comparing the as-written query against the WITH ... WHERE row pipeline is a nice oracle. The coverage of edge props, labels, and node props is good.
• A test with a differing-label chain that hits the antiJoinMap = hopMaps[0] reuse specifically (anchor as source, checkPos > 0) would pin down that second fix. crossLabelChainReusesNoMap may cover it, but it isn't obvious.

LGTM with the minor nits above.

Codecov Report

❌ Patch coverage is 73.91304% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.93%. Comparing base (cbf701d) to head (7d69d5f).

@@            Coverage Diff            @@
##               main    #9288   +/-   ##
=========================================
  Coverage     76.93%   76.93%           
+ Complexity     3982     3980    -2     
=========================================
  Files          2128     2128           
  Lines        214016   214035   +19     
  Branches      45541    45552   +11     
=========================================
+ Hits         164656   164673   +17     
+ Misses        31781    31776    -5     
- Partials      17579    17586    +7     

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

Verified the head 7d69d5f against main cbf701d (both CI jars), on Temurin 25.0.4 with compact object headers and Temurin 21.0.12.

The three fixes do what they say. The cross-label chain (#9277) and the negated pattern's modifiers (#9278) answer like the row pipeline: 0 wrong of 2,700 random chain queries compared with a brute-force count, against 60 wrong before the PR. SUM and AVG over a BYTE property (#9281) give 150 and 75.0 on the unchanged repro, 26 of 26 BYTE-versus-SHORT comparisons agree (11 of 26 before), and Type.increment and Type.decrement are right in 33 of 33 pairs with a Byte (0 of 33 before). LSQB's Q9 text keeps the COUNT ANTI-JOIN CHAIN plan with an unchanged time (ratio of medians 0.99 to 1.04, 10 JVMs per arm, 100 timed runs each, both JDKs).

Two things for the review. The queries the PR declines now take the row pipeline, which is right but slower on the declined spellings (Q9 with an edge property map is 28 to 36x slower, with an end node property map 26 to 27x, with an end label that differs from the chain's 22 to 23x). And a few queries that were right by accident are now wrong: 4 of 3,000 random chains (true count 0, and I think the label bug emptied the frontier and hid a different bug, which is inferred and not tested) and 22 of 2,808 spelled ones (20 negate the chain's own last relationship with a true count of 0, and 2 are the no-inequality shape below).

The push-down is still wrong for other shapes, with the same answers on 26.10.1, on main, and on this head, so they are not about this PR and I would not block on them: #9290 has a deterministic Java repro and the table.