| From: | Junwang Zhao <zhjwpku(at)gmail(dot)com> |
|---|---|
| To: | assam258(at)gmail(dot)com |
| Cc: | zengman <zengman(at)halodbtech(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Amit Langote <amitlangote09(at)gmail(dot)com>, Vik Fearing <vik(at)postgresfriends(dot)org>, Ajay Pal <ajay(dot)pal(dot)k(at)gmail(dot)com>, Imran Zaheer <imran(dot)zhir(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: SQL Property Graph Queries (SQL/PGQ) |
| Date: | 2026-03-18 09:38:20 |
| Message-ID: | CAEG8a3K1+hiFUOavTdncp6kGJEfHit2BozqkWWM1txUGnuP29Q@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Henson,
On Wed, Mar 18, 2026 at 3:21 PM Henson Choi <assam258(at)gmail(dot)com> wrote:
>
> 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?
This fixes the crash.
>
> 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.
One concern is that if we support
MATCH (a IS users)-[]->(x IS users)<-[]-(b IS users WHERE b.name != a.name)
user may expect the following also works:
MATCH (a IS users WHERE b.name != a.name)-[]->(x IS users)<-[]-(b IS users)
but the second actually failed to pass the transform phase.
I tested neo4j, both are well supported.
So we might follow the same behavior. The solution I came out is in
transformPathTerm
we collect gpstate->variables before each transformGraphElementPattern.
Something like:
transformPathTerm(ParseState *pstate, List *path_term)
{
List *result = NIL;
+ GraphTableParseState *gpstate = pstate->p_graph_table_pstate;
+
+ /*
+ * First gather all element variables from this path term so that WHERE
+ * clauses in any element pattern can reference variables
appearing anywhere
+ * in the term, regardless of order.
+ */
+ foreach_node(GraphElementPattern, gep, path_term)
+ {
+ if (gep->variable)
+ {
+ String *v = makeString(pstrdup(gep->variable));
+
+ if (!list_member(gpstate->variables, v))
+ gpstate->variables =
lappend(gpstate->variables, v);
+ }
+ }
foreach_node(GraphElementPattern, gep, path_term)
result = lappend(result,
Thoughts?
>
> Regards,
> Henson
--
Regards
Junwang Zhao
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Álvaro Herrera | 2026-03-18 09:39:34 | Re: [19] CREATE SUBSCRIPTION ... SERVER |
| Previous Message | Daniil Davydov | 2026-03-18 09:23:39 | Re: POC: Parallel processing of indexes in autovacuum |