Latent NULL p_nscolumns in OPTIONAL MATCH lateral-join ParseNamespaceItem
- Dominant language
- C
- Stars
- 4.8k
- Forks
- 523
- Avg merge
- 1d 2h
- Merged PRs (30d)
- 9
Description
Follow-up from #2347. While tracing the MERGE ON CREATE/MATCH SET crash, I confirmed that `transform_cypher_optional_match_clause` at `src/backend/parser/cypher_clause.c:3238` carries the same defect that caused #2347:
```c
jnsitem = addRangeTableEntryForJoin(pstate,
res_colnames,
NULL, /* <-- nscolumns */
j->jointype,
...);
```
Passing NULL for nscolumns leaves the join ParseNamespaceItem's `p_nscolumns` unset. In #2347 this segfaulted whenever `colNameToVar` → `scanNSItemForColumn` tried to resolve a column reference against the nsitem (PG's `scanNSItemForColumn` indexes `nsitem->p_nscolumns[attnum-1]` without null-checking).
I audited the other two `addRangeTableEntryForJoin` call sites in this file (lines 717 and 7564); both pass a real nscolumns array. Line 3238 is the only remaining NULL.
### Why it isn't triggered today
Every current path that would resolve an expression against an OPTIONAL MATCH's output first runs through `handle_prev_clause`, which wraps the prior as a new `RTE_SUBQUERY` with properly-built `p_nscolumns`. The incompletely-populated join nsitem is only in `pstate->p_namespace` during `transform_cypher_optional_match_clause` itself — and no in-line expression resolution happens there today. The jnsitem is also added with `addToRelNameSpace=false` (line 3262), so it's column-visible but not relation-visible, keeping it out of `refnameNamespaceItem` lookups.
### Why it's worth fixing
Any future feature that resolves an in-line expression against the OPTIONAL MATCH's output (e.g. OPTIONAL MATCH analogues of ON SET, or a WHERE processed before the helper returns) will hit the same segfault. The fix is the same three-line change made in #2347's follow-up commit (`f0ed4c95`): populate `p_nscolumns` from `res_colvars`, using the join rtindex and 1-based eref position (Vars inside opaque metadata are not planner-rewritten; see the commit message for context).
### Reproducer
None that triggers the crash from SQL on current master — this is a code-level observation, not a user-visible bug. Recording it so it isn't rediscovered the hard way.
### Proposed fix
Mirror the fix pattern from #2347's `transform_merge_make_lateral_join` change to `transform_cypher_optional_match_clause`. No regression test is feasible until a feature exercises it, but the change is local and the crash mode is well understood.
Happy to submit a PR if maintainers want the defensive fix landed now; otherwise leaving this open so the next person in the neighborhood sees it.
Contributor guide
Assessment
This issue has not been assessed yet.