gregfelice opened a new issue, #2404:
URL: https://github.com/apache/age/issues/2404
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
