Hi Ashutosh, Man,

I reproduced the crash and identified the root cause.


I checked the code and found that `found_mapping` was a null pointer because
>
I didn't enable assertions. The code in question is in
> `src/backend/rewrite/rewriteGraphTable.c`:



Should we remove this assertion and throw an error message instead to
> handle this case?
>

The assertion itself is correct — transformGraphTablePropertyRef() should
never create a GraphPropertyRef for a variable not present in the path
pattern. The real problem is that the element's WHERE clause was only
receiving its own mapping instead of all mappings in the path pattern.

In generate_query_for_graph_path():

  tr = replace_property_refs(rte->relid, pf->whereClause, list_make1(pe));

list_make1(pe) passes only the current element's mapping to
replace_property_refs_mutator(). When the element WHERE clause references
another variable (e.g., `b.name != a.name` inside the `b` element pattern),
the mutator cannot find `a` in the mappings list, leaving found_mapping
NULL.

Note that the graph pattern-level WHERE clause already passes
graph_path (the full mapping list), which is why the same condition works
when placed outside the element pattern.

The fix is simply:

  tr = replace_property_refs(rte->relid, pf->whereClause, graph_path);

Ashutosh, could you include this fix in the next patch revision?

With this fix, the reported query runs without crash and returns the
correct result. The graph_table regression test also passes cleanly.

Also, I'd like to check — do you see any potential side effects from
passing the full graph_path instead of list_make1(pe)? Since the mutator
now has access to all element mappings, I want to make sure there are no
unintended interactions in other code paths.

Regards,
Henson

Reply via email to