Skip to content

Commit d6f1b7f

Browse files
gregfeliceclaude
andauthored
Fix MATCH on brand-new label after CREATE returning 0 rows (#2341)
* Fix MATCH on brand-new label after CREATE returning 0 rows (issue #2193) When CREATE introduces a new label and a subsequent MATCH references it (e.g., CREATE (:Person) WITH ... MATCH (p:Person)), the query returns 0 rows on first execution but works on the second. Root cause: match_check_valid_label() in transform_cypher_match() runs before transform_prev_cypher_clause() processes the predecessor chain. Since CREATE has not yet executed its transform (which creates the label table as a side effect), the label is not in the cache and the check generates a One-Time Filter: false plan that returns no rows. Fix: Skip the early label validity check when the predecessor clause chain contains a data-modifying operation (CREATE, SET, DELETE, MERGE). After transform_prev_cypher_clause() completes and any new labels exist in the cache, run a deferred label check. If the labels are still invalid at that point, generate an empty result via makeBoolConst(false). This preserves the existing behavior for MATCH without DML predecessors (e.g., MATCH-MATCH chains still get the early check and proper error messages for invalid labels). Depends on: PR #2340 (clause_chain_has_dml helper) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address review feedback: fix variable registration for deferred label check When the deferred label validity check (DML predecessor + non-existent label) found an invalid label, the code skipped transform_match_pattern() entirely, which meant MATCH-introduced variables were never registered in the namespace. This would cause errors if a later clause referenced those variables (e.g., RETURN p). Fix: mirror the early-check strategy by injecting a paradoxical WHERE (true = false) and always calling transform_match_pattern(). Variables get registered normally; zero rows are returned via the impossible qual. Also add ORDER BY to multi-row regression tests for deterministic output, and add a test case for DML predecessor + non-existent label + returning a MATCH-introduced variable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address Copilot review: DRY false-where helper, cache has_dml, ORDER BY in tests - Factor duplicated WHERE true=false construction into make_false_where_clause() helper (used in both early and deferred label validation paths) - Compute clause_chain_has_dml() once and reuse, avoiding repeated clause chain traversal - Add ORDER BY to the single-CREATE City regression test for deterministic result ordering * Address Copilot review: volatile false predicate, DML side-effect test 1. Prevent plan elimination of DML predecessor: replace constant (true = false) with volatile (random() IS NULL) in the deferred label check path. PG's planner can constant-fold the former into a One-Time Filter: false, skipping the DML scan entirely. 2. Unify make_false_where_clause(bool volatile_needed): merge the constant and volatile variants into a single parameterized function. Call sites are now self-documenting: - make_false_where_clause(false) for non-DML path - make_false_where_clause(true) for DML predecessor path 3. Document why add_volatile_wrapper() cannot be reused here (it operates post-transform at the Expr level and returns agtype, while the WHERE clause is built at the parse-tree level). 4. Add regression test verifying CREATE side effects persist when MATCH references a non-existent label after a DML predecessor. All regression tests pass (cypher_match: ok). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Replace non-ASCII em dashes with -- in C comments ASCII-only codebase convention; avoids encoding/tooling issues. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a29e281 commit d6f1b7f

3 files changed

Lines changed: 244 additions & 14 deletions

File tree

regress/expected/cypher_match.out

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3737,6 +3737,103 @@ EXECUTE issue_1964_vertex_eq_on('{"props": {"name": "Alice", "age": 25}}');
37373737
(0 rows)
37383738

37393739
DEALLOCATE issue_1964_vertex_eq_on;
3740+
--
3741+
-- Issue 2193: CREATE ... WITH ... MATCH on brand-new label returns 0 rows
3742+
-- on first execution because match_check_valid_label() runs before
3743+
-- transform_prev_cypher_clause() creates the label table.
3744+
--
3745+
SELECT create_graph('issue_2193');
3746+
NOTICE: graph "issue_2193" has been created
3747+
create_graph
3748+
--------------
3749+
3750+
(1 row)
3751+
3752+
-- Reporter's exact case: CREATE two Person nodes, then MATCH on Person
3753+
-- Should return 2 rows on the very first execution
3754+
SELECT * FROM cypher('issue_2193', $$
3755+
CREATE (a:Person {name: 'Jane', livesIn: 'London'}),
3756+
(b:Person {name: 'Tom', livesIn: 'Copenhagen'})
3757+
WITH a, b
3758+
MATCH (p:Person)
3759+
RETURN p.name ORDER BY p.name
3760+
$$) AS (result agtype);
3761+
result
3762+
--------
3763+
"Jane"
3764+
"Tom"
3765+
(2 rows)
3766+
3767+
-- Single CREATE + MATCH on brand-new label
3768+
SELECT * FROM cypher('issue_2193', $$
3769+
CREATE (a:City {name: 'Berlin'})
3770+
WITH a
3771+
MATCH (c:City)
3772+
RETURN c.name ORDER BY c.name
3773+
$$) AS (result agtype);
3774+
result
3775+
----------
3776+
"Berlin"
3777+
(1 row)
3778+
3779+
-- MATCH on a label that now exists (second execution) still works
3780+
SELECT * FROM cypher('issue_2193', $$
3781+
CREATE (a:City {name: 'Paris'})
3782+
WITH a
3783+
MATCH (c:City)
3784+
RETURN c.name ORDER BY c.name
3785+
$$) AS (result agtype);
3786+
result
3787+
----------
3788+
"Berlin"
3789+
"Paris"
3790+
(2 rows)
3791+
3792+
-- MATCH on non-existent label without DML predecessor still returns 0 rows
3793+
SELECT * FROM cypher('issue_2193', $$
3794+
MATCH (x:NonExistentLabel)
3795+
RETURN x
3796+
$$) AS (result agtype);
3797+
result
3798+
--------
3799+
(0 rows)
3800+
3801+
-- MATCH on non-existent label after DML predecessor still returns 0 rows
3802+
-- and MATCH-introduced variable (p) is properly registered
3803+
SELECT * FROM cypher('issue_2193', $$
3804+
CREATE (a:Person {name: 'Alice'})
3805+
WITH a
3806+
MATCH (p:NonExistentLabel)
3807+
RETURN p
3808+
$$) AS (result agtype);
3809+
result
3810+
--------
3811+
(0 rows)
3812+
3813+
-- Verify that the CREATE side effect was preserved even though MATCH
3814+
-- returned 0 rows (guards against plan-elimination regressions where
3815+
-- a constant-false predicate causes PG to skip the DML predecessor)
3816+
SELECT * FROM cypher('issue_2193', $$
3817+
MATCH (a:Person {name: 'Alice'})
3818+
RETURN a.name
3819+
$$) AS (result agtype);
3820+
result
3821+
---------
3822+
"Alice"
3823+
(1 row)
3824+
3825+
SELECT drop_graph('issue_2193', true);
3826+
NOTICE: drop cascades to 4 other objects
3827+
DETAIL: drop cascades to table issue_2193._ag_label_vertex
3828+
drop cascades to table issue_2193._ag_label_edge
3829+
drop cascades to table issue_2193."Person"
3830+
drop cascades to table issue_2193."City"
3831+
NOTICE: graph "issue_2193" has been dropped
3832+
drop_graph
3833+
------------
3834+
3835+
(1 row)
3836+
37403837
--
37413838
-- Clean up
37423839
--

regress/sql/cypher_match.sql

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1558,6 +1558,64 @@ PREPARE issue_1964_vertex_eq_on(agtype) AS
15581558
EXECUTE issue_1964_vertex_eq_on('{"props": {"name": "Alice", "age": 25}}');
15591559
DEALLOCATE issue_1964_vertex_eq_on;
15601560

1561+
--
1562+
-- Issue 2193: CREATE ... WITH ... MATCH on brand-new label returns 0 rows
1563+
-- on first execution because match_check_valid_label() runs before
1564+
-- transform_prev_cypher_clause() creates the label table.
1565+
--
1566+
SELECT create_graph('issue_2193');
1567+
1568+
-- Reporter's exact case: CREATE two Person nodes, then MATCH on Person
1569+
-- Should return 2 rows on the very first execution
1570+
SELECT * FROM cypher('issue_2193', $$
1571+
CREATE (a:Person {name: 'Jane', livesIn: 'London'}),
1572+
(b:Person {name: 'Tom', livesIn: 'Copenhagen'})
1573+
WITH a, b
1574+
MATCH (p:Person)
1575+
RETURN p.name ORDER BY p.name
1576+
$$) AS (result agtype);
1577+
1578+
-- Single CREATE + MATCH on brand-new label
1579+
SELECT * FROM cypher('issue_2193', $$
1580+
CREATE (a:City {name: 'Berlin'})
1581+
WITH a
1582+
MATCH (c:City)
1583+
RETURN c.name ORDER BY c.name
1584+
$$) AS (result agtype);
1585+
1586+
-- MATCH on a label that now exists (second execution) still works
1587+
SELECT * FROM cypher('issue_2193', $$
1588+
CREATE (a:City {name: 'Paris'})
1589+
WITH a
1590+
MATCH (c:City)
1591+
RETURN c.name ORDER BY c.name
1592+
$$) AS (result agtype);
1593+
1594+
-- MATCH on non-existent label without DML predecessor still returns 0 rows
1595+
SELECT * FROM cypher('issue_2193', $$
1596+
MATCH (x:NonExistentLabel)
1597+
RETURN x
1598+
$$) AS (result agtype);
1599+
1600+
-- MATCH on non-existent label after DML predecessor still returns 0 rows
1601+
-- and MATCH-introduced variable (p) is properly registered
1602+
SELECT * FROM cypher('issue_2193', $$
1603+
CREATE (a:Person {name: 'Alice'})
1604+
WITH a
1605+
MATCH (p:NonExistentLabel)
1606+
RETURN p
1607+
$$) AS (result agtype);
1608+
1609+
-- Verify that the CREATE side effect was preserved even though MATCH
1610+
-- returned 0 rows (guards against plan-elimination regressions where
1611+
-- a constant-false predicate causes PG to skip the DML predecessor)
1612+
SELECT * FROM cypher('issue_2193', $$
1613+
MATCH (a:Person {name: 'Alice'})
1614+
RETURN a.name
1615+
$$) AS (result agtype);
1616+
1617+
SELECT drop_graph('issue_2193', true);
1618+
15611619
--
15621620
-- Clean up
15631621
--

src/backend/parser/cypher_clause.c

Lines changed: 89 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,7 @@ static bool isa_special_VLE_case(cypher_path *path);
346346
static ParseNamespaceItem *find_pnsi(cypher_parsestate *cpstate, char *varname);
347347
static bool has_list_comp_or_subquery(Node *expr, void *context);
348348
static bool clause_chain_has_dml(cypher_clause *clause);
349+
static Node *make_false_where_clause(bool volatile_needed);
349350

350351
/*
351352
* Add required permissions to the RTEPermissionInfo for a relation.
@@ -2639,20 +2640,19 @@ static Query *transform_cypher_match(cypher_parsestate *cpstate,
26392640
cypher_match *match_self = (cypher_match*) clause->self;
26402641
Node *where = match_self->where;
26412642

2642-
if(!match_check_valid_label(match_self, cpstate))
2643+
/*
2644+
* Check label validity early unless the predecessor clause chain
2645+
* contains a data-modifying operation (CREATE, SET, DELETE, MERGE).
2646+
* DML predecessors may create new labels that are not yet in the
2647+
* cache, so the check is deferred to after transform_prev_cypher_clause()
2648+
* for those cases.
2649+
*/
2650+
if (!clause_chain_has_dml(clause->prev) &&
2651+
!match_check_valid_label(match_self, cpstate))
26432652
{
2644-
cypher_bool_const *l = make_ag_node(cypher_bool_const);
2645-
cypher_bool_const *r = make_ag_node(cypher_bool_const);
2646-
2647-
l->boolean = true;
2648-
l->location = -1;
2649-
r->boolean = false;
2650-
r->location = -1;
2651-
2652-
/*if the label is invalid, create a paradoxical where to get null*/
2653-
match_self->where = (Node *)makeSimpleA_Expr(AEXPR_OP, "=",
2654-
(Node *)l,
2655-
(Node *)r, -1);
2653+
/* Label is invalid -- inject a false WHERE so the MATCH returns
2654+
* zero rows. No DML predecessor here, so constant-foldable is fine. */
2655+
match_self->where = make_false_where_clause(false);
26562656
}
26572657

26582658
if (has_list_comp_or_subquery((Node *)match_self->where, NULL))
@@ -2915,6 +2915,7 @@ static Query *transform_cypher_match_pattern(cypher_parsestate *cpstate,
29152915
RangeTblEntry *rte;
29162916
int rtindex;
29172917
ParseNamespaceItem *pnsi;
2918+
bool has_dml;
29182919

29192920
pnsi = transform_prev_cypher_clause(cpstate, clause->prev, true);
29202921
rte = pnsi->p_rte;
@@ -2928,7 +2929,9 @@ static Query *transform_cypher_match_pattern(cypher_parsestate *cpstate,
29282929
* DML executes -- resulting in quals checking NULL values
29292930
* and filtering out all rows.
29302931
*/
2931-
if (clause_chain_has_dml(clause->prev))
2932+
has_dml = clause_chain_has_dml(clause->prev);
2933+
2934+
if (has_dml)
29322935
{
29332936
rte->security_barrier = true;
29342937
}
@@ -2949,6 +2952,25 @@ static Query *transform_cypher_match_pattern(cypher_parsestate *cpstate,
29492952
*/
29502953
pnsi = get_namespace_item(pstate, rte);
29512954
query->targetList = expandNSItemAttrs(pstate, pnsi, 0, true, -1);
2955+
2956+
/*
2957+
* Now that the predecessor chain is fully transformed and
2958+
* any CREATE-generated labels exist in the cache, check
2959+
* whether the MATCH pattern references valid labels. This
2960+
* deferred check is only needed when the chain has DML,
2961+
* since labels created by CREATE are not in the cache at
2962+
* the time of the early check in transform_cypher_match().
2963+
*
2964+
* We use a volatile false predicate (random() IS NULL)
2965+
* instead of a constant one (true = false) because PG's
2966+
* planner can constant-fold the latter into a One-Time
2967+
* Filter: false, eliminating the entire plan subtree --
2968+
* including the DML predecessor scan -- without executing it.
2969+
*/
2970+
if (has_dml && !match_check_valid_label(self, cpstate))
2971+
{
2972+
where = make_false_where_clause(true);
2973+
}
29522974
}
29532975

29542976
transform_match_pattern(cpstate, query, self->pattern, where);
@@ -6621,6 +6643,59 @@ static bool clause_chain_has_dml(cypher_clause *clause)
66216643
return false;
66226644
}
66236645

6646+
/*
6647+
* Build a false WHERE clause that forces a MATCH to return zero rows.
6648+
* Used when the MATCH pattern references a label that does not exist.
6649+
*
6650+
* When volatile_needed is false, returns a constant (true = false) that
6651+
* PG's planner may constant-fold -- this is fine when there is no DML
6652+
* predecessor whose execution must be preserved.
6653+
*
6654+
* When volatile_needed is true, returns (random() IS NULL) instead.
6655+
* random() is VOLATILE, so eval_const_expressions() cannot fold this,
6656+
* preventing PG from creating a One-Time Filter: false that would
6657+
* eliminate the DML predecessor scan without executing it.
6658+
*
6659+
* Note: AGE's add_volatile_wrapper() serves a similar anti-fold purpose
6660+
* but operates at the Expr level (post-transform) and returns agtype,
6661+
* so it cannot be used here in the parse-tree WHERE clause context.
6662+
*/
6663+
static Node *make_false_where_clause(bool volatile_needed)
6664+
{
6665+
if (volatile_needed)
6666+
{
6667+
FuncCall *random_fn;
6668+
NullTest *nt;
6669+
6670+
random_fn = makeFuncCall(
6671+
list_make2(makeString("pg_catalog"), makeString("random")),
6672+
NIL,
6673+
COERCE_EXPLICIT_CALL,
6674+
-1);
6675+
6676+
nt = makeNode(NullTest);
6677+
nt->arg = (Expr *)random_fn;
6678+
nt->nulltesttype = IS_NULL;
6679+
nt->argisrow = false;
6680+
nt->location = -1;
6681+
6682+
return (Node *)nt;
6683+
}
6684+
else
6685+
{
6686+
cypher_bool_const *l = make_ag_node(cypher_bool_const);
6687+
cypher_bool_const *r = make_ag_node(cypher_bool_const);
6688+
6689+
l->boolean = true;
6690+
l->location = -1;
6691+
r->boolean = false;
6692+
r->location = -1;
6693+
6694+
return (Node *)makeSimpleA_Expr(AEXPR_OP, "=",
6695+
(Node *)l, (Node *)r, -1);
6696+
}
6697+
}
6698+
66246699
static Query *analyze_cypher_clause(transform_method transform,
66256700
cypher_clause *clause,
66266701
cypher_parsestate *parent_cpstate)

0 commit comments

Comments
 (0)