Gate the anti-edge chain count rewrite on native storage

[itz-puneet]

Addresses item 3 of #939. Scoped per @adsharma's note on that issue: focus on the native gate, verify icebug-disk manually for now.

Problem

tryRewriteExtendChainCount requires every participating table to be native before letting the count fast path run — getStorage().empty() && getStorageFormat() == StorageFormat::NONE, applied to rel groups at count_rel_table_optimizer.cpp:598 and to both node entries at :607. The comment there spells out why: the fast path iterates the committed node-group grid of native node tables and reads the CSR of native rel tables, and Arrow-backed and icebug-disk tables have neither.

tryRewriteAntiEdgeChainCount checks only getScanFunction().has_value(), at :237 for the anti rel group and :408 for the prefix-chain extends, and never checks the node entry at all. So the same CSR-grid arithmetic could run against tables that have no such grid.

This is reachable rather than theoretical. bind_ddl.cpp:437 rejects mixing storage formats but explicitly permits an all-icebug-disk graph, and dataset/demo-db/icebug-disk/schema.cypher is exactly that shape — user nodes plus follows(FROM user TO user), the self-referencing pattern this rewrite matches.

Change

Hoist the two isNative*Entry predicates out of tryRewriteExtendChainCount into file-scope helpers, following the existing forward-declare-then-define convention in that file, and apply them in the anti-edge path to:

• the anti rel group,
• the prefix-chain rel groups, and
• the node entry — n0, n1 and n2 all bind midTableID, so one check covers every node the operator's arithmetic touches.

This only narrows when the rewrite fires. Anything it now declines falls back to the regular plan, so the result stays correct either way.

Verification

The optimizer translation unit compiles clean under -Wall -Wextra, matching the unmodified baseline, and clang-format-18 reports no diff. No behavioral test is included, per the scoping above.

Items 1 and 2 of #939 — node visibility inside the operators, and clipping getOffsetUpperBound() to the reader snapshot — are untouched and remain open.

[Copilot]

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

[adsharma]

• Needs a regression test
• Suffix-hop node tables beyond n2 aren't explicitly gated — only their rel groups and the mid node are. Today this relies on the binder invariant
  (native rel ⇒ native endpoints; bind_ddl.cpp:435-440), whereas the extend-chain path gates every node explicitly. Note there's no equivalent mixing
  guard for Arrow tables in bind_ddl.cpp, so this relies on Arrow node tables never being rel endpoints. Consider gating fromNode/toNode entries in the
  suffix-hop loop for full parity and defense-in-depth.

[itz-puneet]

Both addressed in 96d7485.

Suffix-hop endpoints — good catch, and the Arrow point is the part that convinced me: bind_ddl.cpp:435-440 only guards icebug-disk mixing, so a native-looking rel group tells you nothing about Arrow-backed endpoints. The suffix-hop loop now gates fromNode/toNode alongside the rel group, so every node the arithmetic touches is checked rather than just the mid node. That is the same coverage the extend-chain path has.

Regression test — two cases in optimizer_test.cpp over one query. SkipsIcebugDiskTables asserts COUNT_ANTI_EDGE_CHAIN is absent; RewritesNativeTables asserts it is present on equivalent native tables. The second is there on purpose: without it the skip case would keep passing if the query ever stopped matching the rewrite for an unrelated reason, and I would not be able to tell the difference.

I cannot build locally, so CI is the check on both.

[itz-puneet]

@adsharma the failing check is deliberate, and I could use a pointer.

I wrote the regression test as a pair on one query: SkipsIcebugDiskTables asserts COUNT_ANTI_EDGE_CHAIN is absent, and RewritesNativeTables asserts it is present on equivalent native tables. The second exists so the first cannot pass merely because the query stopped matching the rewrite.

That is what is happening. SkipsIcebugDiskTables passes, RewritesNativeTables fails with Actual: false — so the skip assertion is currently vacuous, and I would rather show that than land a test that proves nothing.

What I have tried for the positive case, both on an empty-DB fixture with a small hand-built graph:

• a three-extend chain with NOT (a)-[:follows]-(b) — wrong, extends.size() >= 4 rejects it
• the LSQB q9 shape from test_files/lsqb/lsqb_queries.test:51, i.e. MATCH (u1:user)-[:follows]-(u2:user)-[:follows]-(u3:user)-[:livesin]->(c:city) WHERE NOT EXISTS {MATCH (u1)-[:follows]-(u3)} AND id(u1) <> id(u3) RETURN count(*), over the user/follows/city/livesin schema

I confirmed the rewrite is not behind a flag — visitAggregateReplace calls it unconditionally — so I assume the plan shape the matcher wants (INNER hash join over a MARK join, prefix chain rooted at the scan of the middle node) only emerges under join orders that my six-node graph does not produce.

Is this cardinality-dependent in practice, i.e. does it only show up on something like lsqb-sf01? If so I am happy to move the positive case to a .test file against that dataset, or to drop it and keep only the skip assertion if you would rather not pay for a large fixture — your call on which is worth the CI time. If there is a smaller query you know produces this plan, that would be ideal.

The source changes themselves are unaffected; only this one test is failing.