Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix MERGE variable reuse #997

Merged
merged 1 commit into from
Jun 20, 2023
Merged

Fix MERGE variable reuse #997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions regress/expected/cypher_match.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2174,6 +2174,15 @@ SELECT * FROM cypher('cypher_match', $$ CREATE () WITH * MATCH (x{n0:x.n1}) RETU
---
(0 rows)

-- these should fail due to multiple labels for a variable
SELECT * FROM cypher('cypher_match', $$ MATCH p=(x)-[]->(x:R) RETURN p, x $$) AS (p agtype, x agtype);
ERROR: multiple labels for variable 'x' are not supported
LINE 1: ...* FROM cypher('cypher_match', $$ MATCH p=(x)-[]->(x:R) RETUR...
^
SELECT * FROM cypher('cypher_match', $$ MATCH p=(x:r)-[]->(x:R) RETURN p, x $$) AS (p agtype, x agtype);
ERROR: multiple labels for variable 'x' are not supported
LINE 1: ...FROM cypher('cypher_match', $$ MATCH p=(x:r)-[]->(x:R) RETUR...
^
--
-- Clean up
--
Expand Down
160 changes: 144 additions & 16 deletions regress/expected/cypher_merge.out
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions regress/sql/cypher_match.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,10 @@ SELECT * FROM cypher('cypher_match', $$ MATCH p=(a {name:a.name})-[u {relationsh

SELECT * FROM cypher('cypher_match', $$ CREATE () WITH * MATCH (x{n0:x.n1}) RETURN 0 $$) as (a agtype);

-- these should fail due to multiple labels for a variable
SELECT * FROM cypher('cypher_match', $$ MATCH p=(x)-[]->(x:R) RETURN p, x $$) AS (p agtype, x agtype);
SELECT * FROM cypher('cypher_match', $$ MATCH p=(x:r)-[]->(x:R) RETURN p, x $$) AS (p agtype, x agtype);

--
-- Clean up
--
Expand Down
41 changes: 35 additions & 6 deletions regress/sql/cypher_merge.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,19 +482,48 @@ SELECT * FROM cypher('cypher_merge', $$ MATCH (n:node) RETURN n $$) AS (n agtype
--
-- Complex MERGE w/wo RETURN values
--
-- These should each create a path, if it doesn't already exist.
-- TODO Until the issue with variable reuse of 'x' in MERGE is corrected,
-- these commands will each create a new path.
-- The first one should create a path, the others should just return parts of it.
SELECT * FROM cypher('cypher_merge', $$ MERGE ()-[:B]->(x:C)-[:E]->(x:C)<-[f:F]-(y:I) $$) AS (x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE ()-[:B]->(x:C)-[:E]->(x:C)<-[f:F]-(y:I) RETURN x $$) AS (x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE p=()-[:B]->(x:C)-[:E]->(x:C)<-[f:F]-(y:I) $$) AS (p agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE p=()-[:B]->(x:C)-[:E]->(x:C)<-[f:F]-(y:I) RETURN p $$) AS (p agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE p=()-[:B]->(x:C)-[:E]->(x:C)<-[f:F]-(y:I) RETURN p $$) AS (p agtype);
-- TODO This should only return 1 row, as the path should already exist.
-- However, we need to fix the variable reuse in MERGE. Until then,
-- this will always return 5 rows due to 'x' above not being the same node.

-- This should only return 1 row, as the path should already exist.
SELECT * FROM cypher('cypher_merge', $$ MATCH p=()-[:B]->(:C)-[:E]->(:C)<-[:F]-(:I) RETURN p $$) AS (p agtype);

-- test variable reuse in MERGE - the first MERGE of each group should create,
-- the second MERGE shouldn't.
SELECT * FROM cypher('cypher_merge', $$ MATCH p=(x:P)-[:E]->(x:P) RETURN p, x $$) AS (p agtype, x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE (x:P)-[:E]->(x:P) $$) AS (x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE (x:P)-[:E]->(x) $$) AS (x agtype);
SELECT * FROM cypher('cypher_merge', $$ MATCH p=(x:P)-[:E]->(x) RETURN p, x $$) AS (p agtype, x agtype);
SELECT * FROM cypher('cypher_merge', $$ MATCH p=(x:Q)-[:E]->(x:Q) RETURN p, x $$) AS (p agtype, x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE (x:Q)-[:E]->(x) $$) AS (x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE (x:Q)-[:E]->(x:Q) $$) AS (x agtype);
SELECT * FROM cypher('cypher_merge', $$ MATCH p=(x:Q)-[:E]->(x) RETURN p, x $$) AS (p agtype, x agtype);
SELECT * FROM cypher('cypher_merge', $$ MATCH p=(x:R)-[:E]->(x) RETURN p, x $$) AS (p agtype, x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x:R)-[:E]->(x) RETURN p, x $$) AS (p agtype, x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x:R)-[:E]->(x) RETURN p, x $$) AS (p agtype, x agtype);
SELECT * FROM cypher('cypher_merge', $$ MATCH p=(x:R)-[:E]->(x) RETURN p, x $$) AS (p agtype, x agtype);
-- should return 4 rows
SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x)-[:E]->(x) RETURN p, x $$) AS (p agtype, x agtype);
-- should create 1 row
SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x)-[:E1]->(x) RETURN p, x $$) AS (p agtype, x agtype);
SELECT * FROM cypher('cypher_merge', $$ MATCH p=(x)-[:E1]->(x) RETURN p, x $$) AS (p agtype, x agtype);
-- the following should fail due to multiple labels
SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x)-[:E]->(x:R) RETURN p, x $$) AS (p agtype, x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x:r)-[:E]->(x:R) RETURN p, x $$) AS (p agtype, x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE (x)-[:E]->(x:R) $$) AS (x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE (x:r)-[:E]->(x:R) $$) AS (x agtype);
-- the following should fail due to reuse issues
SELECT * FROM cypher('cypher_merge', $$ MERGE (x:r)-[y:E]->(x)-[y]->(x) $$) AS (x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE (x:r)-[y:E]->(x)-[x]->(y) $$) AS (x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE (x:r)-[y:E]->(x)-[z:E]->(y) $$) AS (x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x:r)-[y:E]->(x)-[p]->(x) $$) AS (x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x:r)-[y:E]->(x)-[p:E]->(x) $$) AS (x agtype);
SELECT * FROM cypher('cypher_merge', $$ MERGE p=(x:r)-[y:E]->(p)-[x]->(y) $$) AS (x agtype);

--clean up
SELECT * FROM cypher('cypher_merge', $$MATCH (n) DETACH DELETE n $$) AS (a agtype);

Expand Down
28 changes: 26 additions & 2 deletions src/backend/executor/cypher_create.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@ static void begin_cypher_create(CustomScanState *node, EState *estate,
cypher_node->id_expr_state =
ExecInitExpr(cypher_node->id_expr, (PlanState *)node);
}

if (cypher_node->prop_expr != NULL)
{
cypher_node->prop_expr_state = ExecInitExpr(cypher_node->prop_expr,
(PlanState *)node);
}
}
}

Expand All @@ -140,7 +146,9 @@ static void begin_cypher_create(CustomScanState *node, EState *estate,
* that have modified the command id.
*/
if (estate->es_output_cid == 0)
{
estate->es_output_cid = estate->es_snapshot->curcid;
}

Increment_Estate_CommandId(estate);
}
Expand Down Expand Up @@ -210,15 +218,19 @@ static TupleTableSlot *exec_cypher_create(CustomScanState *node)
Decrement_Estate_CommandId(estate)
slot = ExecProcNode(node->ss.ps.lefttree);
Increment_Estate_CommandId(estate)

/* break when there are no tuples */
if (TupIsNull(slot))
{
break;
}

/* setup the scantuple that the process_pattern needs */
econtext->ecxt_scantuple =
node->ss.ps.lefttree->ps_ProjInfo->pi_exprContext->ecxt_scantuple;

process_pattern(css);

/*
* This may not be necessary. If we have an empty pattern, nothing was
* inserted and the current command Id was not used. So, only flag it
Expand All @@ -230,6 +242,7 @@ static TupleTableSlot *exec_cypher_create(CustomScanState *node)
used = true;
}
} while (terminal);

/*
* If the current command Id wasn't used, nothing was inserted and we're
* done.
Expand All @@ -238,8 +251,10 @@ static TupleTableSlot *exec_cypher_create(CustomScanState *node)
{
return NULL;
}

/* update the current command Id */
CommandCounterIncrement();

/* if this was a terminal CREATE just return NULL */
if (terminal)
{
Expand All @@ -256,19 +271,25 @@ static void end_cypher_create(CustomScanState *node)
(cypher_create_custom_scan_state *)node;
ListCell *lc;

// increment the command counter
CommandCounterIncrement();

ExecEndNode(node->ss.ps.lefttree);

foreach (lc, css->pattern)
{
cypher_create_path *path = lfirst(lc);
ListCell *lc2;

foreach (lc2, path->target_nodes)
{
cypher_target_node *cypher_node =
(cypher_target_node *)lfirst(lc2);

if (!CYPHER_TARGET_NODE_INSERT_ENTITY(cypher_node->flags))
{
continue;
}

// close all indices for the node
ExecCloseIndices(cypher_node->resultRelInfo);
Expand Down Expand Up @@ -506,8 +527,7 @@ static Datum create_vertex(cypher_create_custom_scan_state *css,
scantuple = ps->ps_ExprContext->ecxt_scantuple;

// make the vertex agtype
result = make_vertex(
id, CStringGetDatum(node->label_name),
result = make_vertex(id, CStringGetDatum(node->label_name),
PointerGetDatum(scanTupleSlot->tts_values[node->prop_attr_num]));

// append to the path list
Expand Down Expand Up @@ -546,9 +566,11 @@ static Datum create_vertex(cypher_create_custom_scan_state *css,
v = get_ith_agtype_value_from_container(&a->root, 0);

if (v->type != AGTV_VERTEX)
{
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("agtype must resolve to a vertex")));
}

// extract the id agtype field
id_value = GET_AGTYPE_VALUE_OBJECT_VALUE(v, "id");
Expand All @@ -570,10 +592,12 @@ static Datum create_vertex(cypher_create_custom_scan_state *css,
if (!SAFE_TO_SKIP_EXISTENCE_CHECK(node->flags))
{
if (!entity_exists(estate, css->graph_oid, DATUM_GET_GRAPHID(id)))
{
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("vertex assigned to variable %s was deleted",
node->variable_name)));
}
}

if (CYPHER_TARGET_NODE_IN_PATH(node->flags))
Expand Down
25 changes: 11 additions & 14 deletions src/backend/executor/cypher_merge.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,27 +198,25 @@ static bool check_path(cypher_merge_custom_scan_state *css,
*/
if (slot->tts_isnull[node->tuple_position - 1])
{

return true;
}
}

}

return false;
}

static void process_path(cypher_merge_custom_scan_state *css)
{
cypher_create_path *path = css->path;

ListCell *lc = list_head(path->target_nodes);

/*
* Create the first vertex. The create_vertex function will
* create the rest of the path, if necessary.
*/
merge_vertex(css, lfirst(lc), lnext(lc));

merge_vertex(css, lfirst(lc), lnext(lc));

/*
* If this path is a variable, take the list that was accumulated
Expand Down Expand Up @@ -272,9 +270,13 @@ static void process_simple_merge(CustomScanState *node)
if (TupIsNull(slot))
{
ExprContext *econtext = node->ss.ps.ps_ExprContext;
SubqueryScanState *sss = (SubqueryScanState *)node->ss.ps.lefttree;

/* our child execution node should be a subquery */
Assert(IsA(sss, SubqueryScanState));

/* setup the scantuple that the process_path needs */
econtext->ecxt_scantuple = node->ss.ps.lefttree->ps_ResultTupleSlot;
econtext->ecxt_scantuple = sss->ss.ss_ScanTupleSlot;
econtext->ecxt_scantuple->tts_isempty = false;

process_path(css);
Expand Down Expand Up @@ -334,7 +336,7 @@ static TupleTableSlot *exec_cypher_merge(CustomScanState *node)
/*
* Case 1: MERGE is not the first clause in the cypher query.
*
* For this case, we need to process all tuples give to us by the
* For this case, we need to process all tuples given to us by the
* previous clause. When we receive a tuple from the previous clause:
* check to see if the left lateral join found the pattern already. If
* it did, we don't need to create the pattern. If the lateral join did
Expand Down Expand Up @@ -377,7 +379,6 @@ static TupleTableSlot *exec_cypher_merge(CustomScanState *node)
return NULL;
}

//return ExecProject(node->ss.ps.ps_ProjInfo);
econtext->ecxt_scantuple = ExecProject(node->ss.ps.lefttree->ps_ProjInfo);
return ExecProject(node->ss.ps.ps_ProjInfo);

Expand Down Expand Up @@ -783,13 +784,9 @@ static Datum merge_vertex(cypher_merge_custom_scan_state *css,
Datum d;
agtype_value *v = NULL;
agtype_value *id_value = NULL;
TupleTableSlot *scantuple = NULL;
PlanState *ps = NULL;

ps = css->css.ss.ps.lefttree;
scantuple = ps->ps_ExprContext->ecxt_scantuple;

if (scantuple->tts_isnull[node->tuple_position - 1])
/* check that the variable isn't NULL */
if (scanTupleSlot->tts_isnull[node->tuple_position - 1])
{
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
Expand All @@ -798,7 +795,7 @@ static Datum merge_vertex(cypher_merge_custom_scan_state *css,
}

/* get the vertex agtype in the scanTupleSlot */
d = scantuple->tts_values[node->tuple_position - 1];
d = scanTupleSlot->tts_values[node->tuple_position - 1];
a = DATUM_GET_AGTYPE_P(d);

/* Convert to an agtype value */
Expand Down
Loading