Skip to content

fix(planner): bind by-value set once in correlated-optional unnest to avoid hanging MERGE/OPTIONAL MATCH with pattern predicates - #1083

Merged
adsharma merged 1 commit into
LadybugDB:mainfrom
chiangchenghsin-hash:fix/merge-pattern-predicate-ub
Oct 1, 2026
Merged

adsharma merged 1 commit into
LadybugDB:mainfrom
chiangchenghsin-hash:fix/merge-pattern-predicate-ub

Conversation

@chiangchenghsin-hash

Copy link
Copy Markdown
Contributor

Hi! Thanks a lot for 0.21.1 — we've been building on it and hit one planner hang that turned out to come from upstream, so we'd like to contribute a fix. Hopefully this is helpful.

Summary

planOptionalMatch's inner-selectivity gate (introduced in 58c10a4) builds constFilteredVars with:

constFilteredVars.insert(collector.getVarNames().begin(),
    collector.getVarNames().end());

Since getVarNames() returns by value, begin() and end() come from two different temporaries. The insert loop then compares iterators across two distinct containers and never terminates (or walks freed memory): planning spins on one core indefinitely, or crashes with SIGSEGV depending on heap layout.

Reproduction

Hangs at planning time (EXPLAIN is enough — no execution needed) whenever the pattern carries a property predicate and the left plan is non-empty:

MATCH (a:person), (b:person) WHERE a.ID = 0 AND b.ID = 5
MERGE (a)-[r:knows {date: date('2022-02-02')}]->(b);

Also reproduces with a node property map on the MERGE pattern:

MATCH (a:person) WHERE a.ID = 0
MERGE (a)-[r:knows]->(b:person {ID: 5});

Telling details that cost us some time:

  • The same shape without a pattern property (MERGE (a)-[r:knows]->(b)) plans fine — the loop body only runs when the clause has pattern predicates.
  • A pure MERGE with no preceding MATCH also plans fine — the gate is skipped for an empty left plan.
  • On some builds/heap layouts an empty table can appear to pass, which makes the failure look data-dependent even though it is pure UB in planning.

We bisected this to the 58c10a4 window and confirmed the bug is present on current main (file blob 9b269c4).

Fix

Bind the by-value temporary once before taking the iterator range (plus a brief comment so it doesn't come back):

auto predVarNames = collector.getVarNames();
constFilteredVars.insert(predVarNames.begin(), predVarNames.end());

Testing

  • The minimal repro above now plans and executes correctly (checked with EXPLAIN and without).
  • Our full dml_rel/merge, dml_node/merge, and transaction merge e2e suites pass (26 tests, previously hanging on the pattern-property cases).
  • No behavior change for previously passing queries — only the UB is removed.

Happy to adjust anything — thanks for taking a look!

planOptionalMatch's inner-selectivity gate built constFilteredVars with

    constFilteredVars.insert(collector.getVarNames().begin(),
        collector.getVarNames().end());

Because getVarNames() returns by value, begin() and end() came from two
different temporaries. The insert loop then compared iterators across two
containers and never terminated (or walked freed memory): planning hung
on one core, or crashed with SIGSEGV depending on heap layout.

Every MERGE / OPTIONAL MATCH whose pattern carried a property predicate
against a non-empty outer plan hit this, e.g.

    MATCH (a:person), (b:person) WHERE a.ID = 0 AND b.ID = 5
    MERGE (a)-[r:knows {date: date('2022-02-02')}]->(b);

The same shape without a pattern property planned fine, and an empty
table could appear to pass, which made the failure look data-dependent.

Binding the temporary once restores a well-defined iterator range.
Introduced in 58c10a4.
@adsharma adsharma self-assigned this Sep 30, 2026
@adsharma

adsharma commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Thanks!

@adsharma
adsharma merged commit 37bf0dc into LadybugDB:main Oct 1, 2026
4 checks passed
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