apache / apache/age

Latent NULL p_nscolumns in OPTIONAL MATCH lateral-join ParseNamespaceItem

Open
#2,404 3 comments 1 reaction 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.