You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@age.apache.org by jg...@apache.org on 2021/08/18 00:34:53 UTC
[incubator-age] branch master updated: Fix bug in openCypher CREATE
AGE2-337
This is an automated email from the ASF dual-hosted git repository.
jgemignani pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-age.git
The following commit(s) were added to refs/heads/master by this push:
new 7c990fa Fix bug in openCypher CREATE AGE2-337
7c990fa is described below
commit 7c990fa71b01e3ea1456938673237a60cd244994
Author: John Gemignani <jr...@gmail.com>
AuthorDate: Fri Aug 13 17:37:23 2021 -0700
Fix bug in openCypher CREATE AGE2-337
This fixes a bug in the openCypher execution logic for the CREATE
command. See Jira ticket AGE2-337 for more details.
This bug became obvious when using the openCypher CREATE command
as part of an INSERT INTO command. There may be other similar
commands that were affected that this ticket might resolve. This
is due to how the information being passed from the openCypher
SELECT command is passed back into other commands.
This fix involves saving and restoring the result relation info for
the current target node state. This is done in both the create_vertex
and create_edge functions.
The previous logic just overwrote the value. However, when tuples
were passed back, say to INSERT, this modified value did not align
with what the value actually was. This caused crashes as the data
had the wrong descriptor. It may have also cause silent bugs.
Added regression tests for INSERT INTO (...)
---
regress/expected/cypher_create.out | 17 +++++++++++-
regress/sql/cypher_create.sql | 11 ++++++++
src/backend/executor/cypher_create.c | 50 ++++++++++++++++++++++++++----------
3 files changed, 63 insertions(+), 15 deletions(-)
diff --git a/regress/expected/cypher_create.out b/regress/expected/cypher_create.out
index dcb08fb..a917376 100644
--- a/regress/expected/cypher_create.out
+++ b/regress/expected/cypher_create.out
@@ -579,11 +579,25 @@ ERROR: label existing_elabel is for edges, not vertices
LINE 2: CREATE (a:existing_elabel { id: 5})
^
--
+-- check the cypher CREATE clause inside an INSERT INTO
+--
+CREATE TABLE simple_path (u agtype, e agtype, v agtype);
+INSERT INTO simple_path(SELECT * FROM cypher('cypher_create',
+ $$CREATE (u)-[e:knows]->(v) return u, e, v
+ $$) AS (u agtype, e agtype, v agtype));
+SELECT count(*) FROM simple_path;
+ count
+-------
+ 1
+(1 row)
+
+--
-- Clean up
--
+DROP TABLE simple_path;
DROP FUNCTION create_test;
SELECT drop_graph('cypher_create', true);
-NOTICE: drop cascades to 11 other objects
+NOTICE: drop cascades to 12 other objects
DETAIL: drop cascades to table cypher_create._ag_label_vertex
drop cascades to table cypher_create._ag_label_edge
drop cascades to table cypher_create.v
@@ -595,6 +609,7 @@ drop cascades to table cypher_create.b_var
drop cascades to table cypher_create.new_vertex
drop cascades to table cypher_create.existing_vlabel
drop cascades to table cypher_create.existing_elabel
+drop cascades to table cypher_create.knows
NOTICE: graph "cypher_create" has been dropped
drop_graph
------------
diff --git a/regress/sql/cypher_create.sql b/regress/sql/cypher_create.sql
index e59e960..a3642b7 100644
--- a/regress/sql/cypher_create.sql
+++ b/regress/sql/cypher_create.sql
@@ -274,8 +274,19 @@ SELECT * FROM cypher('cypher_create', $$
$$) as (a agtype);
--
+-- check the cypher CREATE clause inside an INSERT INTO
+--
+CREATE TABLE simple_path (u agtype, e agtype, v agtype);
+
+INSERT INTO simple_path(SELECT * FROM cypher('cypher_create',
+ $$CREATE (u)-[e:knows]->(v) return u, e, v
+ $$) AS (u agtype, e agtype, v agtype));
+
+SELECT count(*) FROM simple_path;
+--
-- Clean up
--
+DROP TABLE simple_path;
DROP FUNCTION create_test;
SELECT drop_graph('cypher_create', true);
diff --git a/src/backend/executor/cypher_create.c b/src/backend/executor/cypher_create.c
index edf2d0d..9d8f89a 100644
--- a/src/backend/executor/cypher_create.c
+++ b/src/backend/executor/cypher_create.c
@@ -324,6 +324,7 @@ static void create_edge(cypher_create_custom_scan_state *css,
EState *estate = css->css.ss.ps.state;
ExprContext *econtext = css->css.ss.ps.ps_ExprContext;
ResultRelInfo *resultRelInfo = node->resultRelInfo;
+ ResultRelInfo *old_estate_es_result_relation_info = NULL;
TupleTableSlot *elemTupleSlot = node->elemTupleSlot;
TupleTableSlot *scanTupleSlot = econtext->ecxt_scantuple;
Datum id;
@@ -368,6 +369,10 @@ static void create_edge(cypher_create_custom_scan_state *css,
*
* Note: This obliterates what was their previously
*/
+
+ /* save the old result relation info */
+ old_estate_es_result_relation_info = estate->es_result_relation_info;
+
estate->es_result_relation_info = resultRelInfo;
ExecClearTuple(elemTupleSlot);
@@ -394,6 +399,9 @@ static void create_edge(cypher_create_custom_scan_state *css,
// Insert the new edge
insert_entity_tuple(resultRelInfo, elemTupleSlot, estate);
+ /* restore the old result relation info */
+ estate->es_result_relation_info = old_estate_es_result_relation_info;
+
/*
* When the edge is used by clauses higher in the execution tree
* we need to create an edge datum. When the edge is a variable,
@@ -447,12 +455,18 @@ static Datum create_vertex(cypher_create_custom_scan_state *css,
*/
if (CYPHER_TARGET_NODE_INSERT_ENTITY(node->flags))
{
+ ResultRelInfo *old_estate_es_result_relation_info = NULL;
+
/*
* Set estate's result relation to the vertex's result
* relation.
*
* Note: This obliterates what was their previously
*/
+
+ /* save the old result relation info */
+ old_estate_es_result_relation_info = estate->es_result_relation_info;
+
estate->es_result_relation_info = resultRelInfo;
ExecClearTuple(elemTupleSlot);
@@ -471,6 +485,9 @@ static Datum create_vertex(cypher_create_custom_scan_state *css,
// Insert the new vertex
insert_entity_tuple(resultRelInfo, elemTupleSlot, estate);
+ /* restore the old result relation info */
+ estate->es_result_relation_info = old_estate_es_result_relation_info;
+
/*
* When the vertex is used by clauses higher in the execution tree
* we need to create a vertex datum. When the vertex is a variable,
@@ -494,12 +511,13 @@ static Datum create_vertex(cypher_create_custom_scan_state *css,
// append to the path list
if (CYPHER_TARGET_NODE_IN_PATH(node->flags))
{
- css->path_values = lappend(css->path_values, DatumGetPointer(result));
+ css->path_values = lappend(css->path_values,
+ DatumGetPointer(result));
}
/*
- * Put the vertex in the correct spot in the scantuple, so parent execution
- * nodes can reference the newly created variable.
+ * Put the vertex in the correct spot in the scantuple, so parent
+ * execution nodes can reference the newly created variable.
*/
if (CYPHER_TARGET_NODE_IS_VARIABLE(node->flags))
{
@@ -537,27 +555,30 @@ static Datum create_vertex(cypher_create_custom_scan_state *css,
id = GRAPHID_GET_DATUM(id_value->val.int_value);
/*
- * Its possible the variable has already been deleted. There are two ways
- * this can happen. One is the query explicitly deleted the variable, the
- * is_deleted flag will catch that. However, it is possible the user deleted
- * the vertex using another variable name. We need to scan the table to find
- * the vertex's current status relative to this CREATE clause. If the variable
- * was initially created in this clause, we can skip this check, because the
- * transaction system guarantees that nothing can happen to that tuple, as
- * far as we are concerned with at this time.
+ * Its possible the variable has already been deleted. There are two
+ * ways this can happen. One is the query explicitly deleted the
+ * variable, the is_deleted flag will catch that. However, it is
+ * possible the user deleted the vertex using another variable name. We
+ * need to scan the table to find the vertex's current status relative
+ * to this CREATE clause. If the variable was initially created in this
+ * clause, we can skip this check, because the transaction system
+ * guarantees that nothing can happen to that tuple, as far as we are
+ * concerned with at this time.
*/
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)));
+ errmsg("vertex assigned to variable %s was deleted",
+ node->variable_name)));
}
if (CYPHER_TARGET_NODE_IN_PATH(node->flags))
{
Datum vertex = scanTupleSlot->tts_values[node->tuple_position - 1];
- css->path_values = lappend(css->path_values, DatumGetPointer(vertex));
+ css->path_values = lappend(css->path_values,
+ DatumGetPointer(vertex));
}
}
@@ -616,7 +637,8 @@ static bool entity_exists(EState *estate, Oid graph_oid, graphid id)
* constraints have not been violated.
*/
static HeapTuple insert_entity_tuple(ResultRelInfo *resultRelInfo,
- TupleTableSlot *elemTupleSlot, EState *estate)
+ TupleTableSlot *elemTupleSlot,
+ EState *estate)
{
HeapTuple tuple;