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/01/04 18:40:08 UTC

[incubator-age] branch master updated: Add REMOVE clause.

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 bab98d9  Add REMOVE clause.
bab98d9 is described below

commit bab98d97ae34f302430e2ec588a44a4ab0d8b39e
Author: John Gemignani <jr...@gmail.com>
AuthorDate: Mon Jan 4 10:39:12 2021 -0800

    Add REMOVE clause.
    
    Added REMOVE clause, remove is a special case of the set clause
    where we set the property to NULL.
    
    Committed for Josh Innis due to issues on his end.
---
 Makefile                           |   1 +
 regress/expected/cypher_set.out    |  40 ++++++-------
 regress/sql/cypher_set.sql         |   1 -
 src/backend/executor/cypher_set.c  |  50 ++++++++++++----
 src/backend/parser/cypher_clause.c | 116 ++++++++++++++++++++++++++++++++++---
 src/backend/parser/cypher_gram.y   |   4 --
 src/include/nodes/cypher_nodes.h   |   5 ++
 7 files changed, 173 insertions(+), 44 deletions(-)

diff --git a/Makefile b/Makefile
index 57cb451..e1c134c 100644
--- a/Makefile
+++ b/Makefile
@@ -63,6 +63,7 @@ REGRESS = scan \
           cypher_create \
           cypher_match \
           cypher_set \
+          cypher_remove \
           cypher_with \
           drop
 
diff --git a/regress/expected/cypher_set.out b/regress/expected/cypher_set.out
index 6f631e9..4c273ad 100644
--- a/regress/expected/cypher_set.out
+++ b/regress/expected/cypher_set.out
@@ -173,14 +173,18 @@ SELECT * FROM cypher('cypher_set', $$
         SET n.z = 99
         RETURN n
 $$) AS (a agtype);
-ERROR:  Entity could not be locked for updating
+                                                    a                                                     
+----------------------------------------------------------------------------------------------------------
+ {"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5, "y": 50, "z": 99}}::vertex
+(1 row)
+
 SELECT * FROM cypher('cypher_set', $$
         MATCH (n {j: 5})
         RETURN n
 $$) AS (a agtype);
-                                           a                                            
-----------------------------------------------------------------------------------------
- {"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5}}::vertex
+                                                    a                                                     
+----------------------------------------------------------------------------------------------------------
+ {"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5, "y": 50, "z": 99}}::vertex
 (1 row)
 
 --Create a loop and see that set can work after create
@@ -190,9 +194,9 @@ SELECT * FROM cypher('cypher_set', $$
 	SET n.y = 99
 	RETURN n, p
 $$) AS (a agtype, b agtype);
-                                                a                                                |                                                                                                                                                                    b                                                                                                                                                                    
--------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
- {"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5, "y": 99}}::vertex | [{"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5, "y": 99}}::vertex, {"id": 1125899906842633, "label": "e", "end_id": 844424930131970, "start_id": 844424930131970, "properties": {"j": 34}}::edge, {"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5, "y": 99}}::vertex]::path
+                                                    a                                                     |                                                                                                                                                                             b                                                                                                                                                                             
+----------------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ {"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5, "y": 99, "z": 99}}::vertex | [{"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5, "y": 99, "z": 99}}::vertex, {"id": 1125899906842633, "label": "e", "end_id": 844424930131970, "start_id": 844424930131970, "properties": {"j": 34}}::edge, {"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5, "y": 99, "z": 99}}::vertex]::path
 (1 row)
 
 --Create a loop and see that set can work after create
@@ -212,10 +216,10 @@ SELECT * FROM cypher('cypher_set', $$
         SET n.y = 1
         RETURN n
 $$) AS (a agtype);
-                                               a                                                
-------------------------------------------------------------------------------------------------
+                                                    a                                                    
+---------------------------------------------------------------------------------------------------------
  {"id": 281474976710658, "label": "", "properties": {"y": 1}}::vertex
- {"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5, "y": 1}}::vertex
+ {"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5, "y": 1, "z": 99}}::vertex
 (2 rows)
 
 SELECT * FROM cypher('cypher_set', $$
@@ -224,16 +228,16 @@ SELECT * FROM cypher('cypher_set', $$
         SET n.y = 2
         RETURN n
 $$) AS (a agtype);
-                                               a                                                
-------------------------------------------------------------------------------------------------
+                                                    a                                                    
+---------------------------------------------------------------------------------------------------------
  {"id": 281474976710659, "label": "", "properties": {"y": 2}}::vertex
- {"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5, "y": 2}}::vertex
+ {"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5, "y": 2, "z": 99}}::vertex
 (2 rows)
 
 SELECT * FROM cypher('cypher_set', $$MATCH (n)-[]->(n) SET n.y = 99 RETURN n$$) AS (a agtype);
-                                                a                                                
--------------------------------------------------------------------------------------------------
- {"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5, "y": 99}}::vertex
+                                                    a                                                     
+----------------------------------------------------------------------------------------------------------
+ {"id": 844424930131970, "label": "v", "properties": {"a": 0, "i": 50, "j": 5, "y": 99, "z": 99}}::vertex
 (1 row)
 
 SELECT * FROM cypher('cypher_set', $$MATCH (n) MATCH (n)-[]->(m) SET n.t = 150 RETURN n$$) AS (a agtype);
@@ -251,10 +255,6 @@ SELECT * FROM cypher('cypher_set', $$MATCH (n) SET n.i = 3, n.j = 5 $$) AS (a ag
 ERROR:  SET clause does not yet support updating more than one property
 LINE 1: SELECT * FROM cypher('cypher_set', $$MATCH (n) SET n.i = 3, ...
                                                        ^
-SELECT * FROM cypher('cypher_set', $$MATCH (n) REMOVE n.i $$) AS (a agtype);
-ERROR:  REMOVE clause not implemented
-LINE 1: SELECT * FROM cypher('cypher_set', $$MATCH (n) REMOVE n.i $$...
-                                                       ^
 --
 -- Clean up
 --
diff --git a/regress/sql/cypher_set.sql b/regress/sql/cypher_set.sql
index df4806d..c68c35c 100644
--- a/regress/sql/cypher_set.sql
+++ b/regress/sql/cypher_set.sql
@@ -100,7 +100,6 @@ SELECT * FROM cypher('cypher_set', $$MATCH (n) SET wrong_var.i = 3$$) AS (a agty
 
 SELECT * FROM cypher('cypher_set', $$MATCH (n) SET n.i = 3, n.j = 5 $$) AS (a agtype);
 
-SELECT * FROM cypher('cypher_set', $$MATCH (n) REMOVE n.i $$) AS (a agtype);
 --
 -- Clean up
 --
diff --git a/src/backend/executor/cypher_set.c b/src/backend/executor/cypher_set.c
index 81494ba..9f3b186 100644
--- a/src/backend/executor/cypher_set.c
+++ b/src/backend/executor/cypher_set.c
@@ -47,6 +47,8 @@ static void rescan_cypher_set(CustomScanState *node);
 
 static void process_update_list(CustomScanState *node);
 agtype_value *alter_property_value(agtype_value *properties, char *var_name, agtype *new_v, bool remove_property);
+static HeapTuple update_entity_tuple(ResultRelInfo *resultRelInfo,
+                                TupleTableSlot *elemTupleSlot, EState *estate, HeapTuple old_tuple);
 
 const CustomExecMethods cypher_set_exec_methods = {"Cypher Set",
                                                       begin_cypher_set,
@@ -89,7 +91,7 @@ static void begin_cypher_set(CustomScanState *node, EState *estate,
     estate->es_output_cid++;
 }
 
-static void update_entity_tuple(ResultRelInfo *resultRelInfo,
+static HeapTuple update_entity_tuple(ResultRelInfo *resultRelInfo,
                                 TupleTableSlot *elemTupleSlot, EState *estate, HeapTuple old_tuple)
 {
     HeapTuple tuple;
@@ -140,6 +142,8 @@ static void update_entity_tuple(ResultRelInfo *resultRelInfo,
     ReleaseBuffer(buffer);
 
     estate->es_result_relation_info = saved_resultRelInfo;
+
+    return tuple;
 }
 
 /*
@@ -148,10 +152,14 @@ static void update_entity_tuple(ResultRelInfo *resultRelInfo,
  */
 static void process_all_tuples(CustomScanState *node)
 {
+    cypher_set_custom_scan_state *css =
+        (cypher_set_custom_scan_state *)node;
     TupleTableSlot *slot;
 
     do
     {
+        css->tuple_info = NIL;
+
         process_update_list(node);
 
         slot = ExecProcNode(node->ss.ps.lefttree);
@@ -275,6 +283,8 @@ static void process_update_list(CustomScanState *node)
         ResultRelInfo *resultRelInfo;
         TupleTableSlot *elemTupleSlot;
         HeapTuple heap_tuple;
+        HeapTuple tuple;
+        char *clause_name = css->set_list->clause_name;
 
         update_item = (cypher_update_item *)lfirst(lc);
 
@@ -287,14 +297,14 @@ static void process_update_list(CustomScanState *node)
 
         if (scanTupleSlot->tts_tupleDescriptor->attrs[update_item->entity_position -1].atttypid != AGTYPEOID)
             ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                        errmsg("age set clause can only update agtype")));
+                    errmsg("age %s clause can only update agtype", clause_name)));
 
         original_entity = DATUM_GET_AGTYPE_P(scanTupleSlot->tts_values[update_item->entity_position - 1]);
         original_entity_value = get_ith_agtype_value_from_container(&original_entity->root, 0);
 
         if (original_entity_value->type != AGTV_VERTEX && original_entity_value->type != AGTV_EDGE)
             ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                        errmsg("age set clause can only update vertex and edges")));
+                    errmsg("age %s clause can only update vertex and edges", clause_name)));
 
         // get the id and label for later
         id = get_agtype_value_object_value(original_entity_value, "id");
@@ -306,9 +316,11 @@ static void process_update_list(CustomScanState *node)
 
         /*
          * Determine if the property should be removed.
-         * XXX: this will be expanded when the REMOVE clause is implemented.
          */
-        remove_property = scanTupleSlot->tts_isnull[update_item->prop_position - 1];
+        if(update_item->remove_item)
+            remove_property = true;
+        else
+            remove_property = scanTupleSlot->tts_isnull[update_item->prop_position - 1];
 
         if (remove_property)
             new_property_value = NULL;
@@ -350,8 +362,20 @@ static void process_update_list(CustomScanState *node)
         // update the on-disc table
         heap_tuple = get_heap_tuple(node, update_item->var_name);
 
-        // update the on-disc table
-        update_entity_tuple(resultRelInfo, elemTupleSlot, estate, heap_tuple);
+        tuple = update_entity_tuple(resultRelInfo, elemTupleSlot, estate, heap_tuple);
+
+        if (update_item->var_name != NULL)
+        {
+            clause_tuple_information *tuple_info;
+
+            tuple_info = palloc(sizeof(clause_tuple_information));
+
+            tuple_info->tuple = tuple;
+            tuple_info->name = update_item->var_name;
+
+            css->tuple_info = lappend(css->tuple_info, tuple_info);
+        }
+
 
         ExecCloseIndices(resultRelInfo);
 
@@ -375,6 +399,8 @@ static TupleTableSlot *exec_cypher_set(CustomScanState *node)
 
     saved_resultRelInfo = estate->es_result_relation_info;
 
+    css->tuple_info = NIL;
+
     //Process the subtree first
     estate->es_output_cid--;
     slot = ExecProcNode(node->ss.ps.lefttree);
@@ -414,9 +440,13 @@ static void end_cypher_set(CustomScanState *node)
 
 static void rescan_cypher_set(CustomScanState *node)
 {
-    ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                    errmsg("cypher set clause cannot be rescaned"),
-                    errhint("its unsafe to use joins in a query with a Cypher SET clause")));
+    cypher_set_custom_scan_state *css =
+        (cypher_set_custom_scan_state *)node;
+    char *clause_name = css->set_list->clause_name;
+
+     ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                    errmsg("cypher %s clause cannot be rescaned", clause_name),
+                    errhint("its unsafe to use joins in a query with a Cypher %s clause", clause_name)));
 }
 
 Node *create_cypher_set_plan_state(CustomScan *cscan)
diff --git a/src/backend/parser/cypher_clause.c b/src/backend/parser/cypher_clause.c
index 1752c78..3c1529f 100644
--- a/src/backend/parser/cypher_clause.c
+++ b/src/backend/parser/cypher_clause.c
@@ -202,11 +202,12 @@ static TargetEntry *placeholder_target_entry(cypher_parsestate *cpstate,
                                              char *name);
 static Query *transform_cypher_sub_pattern(cypher_parsestate *cpstate,
                                            cypher_clause *clause);
-// set clause
+// set and remove clause
 static Query *transform_cypher_set(cypher_parsestate *cpstate,
                                    cypher_clause *clause);
 cypher_update_information *transform_cypher_set_item_list(cypher_parsestate *cpstate, List *set_item_list, Query *query);
-
+cypher_update_information *transform_cypher_remove_item_list(
+    cypher_parsestate *cpstate, List *remove_item_list, Query *query);
 
 // transform
 #define PREV_CYPHER_CLAUSE_ALIAS "_"
@@ -271,20 +272,21 @@ static Query *transform_cypher_set(cypher_parsestate *cpstate,
     Oid func_set_oid;
     Const *pattern_const;
     Expr *func_expr;
+    char *clause_name;
 
     query = makeNode(Query);
     query->commandType = CMD_SELECT;
     query->targetList = NIL;
 
-    if (self->is_remove)
-        ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-            errmsg("REMOVE clause not yet implemented"),
-            parser_errposition(pstate, self->location)));
+    if (self->is_remove == true)
+        clause_name = UPDATE_CLAUSE_REMOVE;
+    else
+        clause_name = UPDATE_CLAUSE_SET;
 
     if (!clause->prev)
     {
         ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                        errmsg("SET cannot be the first clause in a Cypher query"),
+                        errmsg("%s cannot be the first clause in a Cypher query", clause_name),
                         parser_errposition(pstate, self->location)));
     }
     else
@@ -302,10 +304,21 @@ static Query *transform_cypher_set(cypher_parsestate *cpstate,
 
     if (list_length(self->items) != 1)
         ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-            errmsg("SET clause does not yet support updating more than one property"),
+            errmsg("%s clause does not yet support updating more than one property", clause_name),
+            parser_errposition(pstate, self->location)));
+
+    if (self->is_remove == true)
+        set_items_target_list = transform_cypher_remove_item_list(cpstate, self->items, query);
+    else
+        set_items_target_list = transform_cypher_set_item_list(cpstate, self->items, query);
+
+    if (list_length(self->items) != 1)
+        ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+            errmsg("%s clause does not yet support updating more than one property",
+                    set_items_target_list->clause_name),
             parser_errposition(pstate, self->location)));
 
-    set_items_target_list = transform_cypher_set_item_list(cpstate, self->items, query);
+    set_items_target_list->clause_name = clause_name;
     set_items_target_list->graph_name = cpstate->graph_name;
 
     if (!clause->next)
@@ -329,6 +342,89 @@ static Query *transform_cypher_set(cypher_parsestate *cpstate,
     return query;
 }
 
+cypher_update_information *transform_cypher_remove_item_list(
+    cypher_parsestate *cpstate, List *remove_item_list, Query *query)
+{
+    ParseState *pstate = (ParseState *)cpstate;
+    ListCell *li;
+    cypher_update_information *info = palloc0(sizeof(cypher_update_information));
+
+    info->set_items = NIL;
+    info->flags = 0;
+
+    foreach (li, remove_item_list)
+    {
+        cypher_set_item *set_item = lfirst(li);
+        cypher_update_item *item;
+        ColumnRef *ref;
+        A_Indirection *ind;
+        char *variable_name, *property_name;
+        Value *property_node, *variable_node;
+
+        item = palloc0(sizeof(cypher_update_item));
+
+        if (!is_ag_node(lfirst(li), cypher_set_item))
+            ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                errmsg("unexpected node in cypher update list")));
+
+        if (set_item->is_add)
+           ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                errmsg("REMOVE clause does not support adding propereties from maps"),
+                parser_errposition(pstate, set_item->location)));
+
+        item->remove_item = true;
+
+
+        if (!IsA(set_item->prop, A_Indirection))
+           ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                errmsg("REMOVE clause must be in the format: REMOVE variable.property_name"),
+                parser_errposition(pstate, set_item->location)));
+
+        ind = (A_Indirection *)set_item->prop;
+
+        // extract variable name
+        if (!IsA(ind->arg, ColumnRef))
+           ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                errmsg("REMOVE clause must be in the format: REMOVE variable.property_name"),
+                parser_errposition(pstate, set_item->location)));
+
+        ref = (ColumnRef *)ind->arg;
+
+        variable_node = linitial(ref->fields);
+
+        variable_name = variable_node->val.str;
+        item->var_name = variable_name;
+        item->entity_position = get_target_entry_resno(query->targetList, variable_name);
+
+        if (item->entity_position == -1)
+            ereport(ERROR, (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
+                errmsg("undefined reference to variable %s in REMOVE clause", variable_name),
+                parser_errposition(pstate, set_item->location)));
+
+        // extract property name
+        if (list_length(ind->indirection) != 1)
+           ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                errmsg("REMOVE clause must be in the format: REMOVE variable.property_name"),
+                parser_errposition(pstate, set_item->location)));
+
+        property_node = linitial(ind->indirection);
+
+
+        if (!IsA(property_node, String))
+           ereport(ERROR, (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
+                errmsg("REMOVE clause expects a property name"),
+                parser_errposition(pstate, set_item->location)));
+
+        property_name = property_node->val.str;
+        item->prop_name = property_name;
+
+        info->set_items = lappend(info->set_items, item);
+    }
+
+    return info;
+}
+
+
 cypher_update_information *transform_cypher_set_item_list(
     cypher_parsestate *cpstate, List *set_item_list, Query *query)
 {
@@ -360,6 +456,8 @@ cypher_update_information *transform_cypher_set_item_list(
                 errmsg("SET clause does not yet support adding propereties from maps"),
                 parser_errposition(pstate, set_item->location)));
 
+        item->remove_item = false;
+
         // extract variable name
         ref = (ColumnRef *)ind->arg;
 
diff --git a/src/backend/parser/cypher_gram.y b/src/backend/parser/cypher_gram.y
index ea3cda2..d7ae045 100644
--- a/src/backend/parser/cypher_gram.y
+++ b/src/backend/parser/cypher_gram.y
@@ -609,10 +609,6 @@ remove:
              n->location = @1;
 
             $$ = (Node *)n;
-
-            ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                            errmsg("REMOVE clause not implemented"),
-                            ag_scanner_errposition(@1, scanner)));
         }
     ;
 
diff --git a/src/include/nodes/cypher_nodes.h b/src/include/nodes/cypher_nodes.h
index de02620..4c051cd 100644
--- a/src/include/nodes/cypher_nodes.h
+++ b/src/include/nodes/cypher_nodes.h
@@ -268,6 +268,9 @@ typedef struct cypher_create_path
 #define CYPHER_TARGET_NODE_INSERT_ENTITY(flags) \
     (flags & CYPHER_TARGET_NODE_FLAG_INSERT)
 
+#define UPDATE_CLAUSE_SET "SET"
+#define UPDATE_CLAUSE_REMOVE "REMOVE"
+
 /* Data Structures that contain information about a vertices and edges the need to be updated */
 typedef struct cypher_update_information
 {
@@ -275,6 +278,7 @@ typedef struct cypher_update_information
     int flags;
     AttrNumber tuple_position;
     char *graph_name;
+    char *clause_name;
 } cypher_update_information;
 
 typedef struct cypher_update_item
@@ -284,6 +288,7 @@ typedef struct cypher_update_item
     char *var_name;
     char *prop_name;
     List *qualified_name;
+    bool remove_item;
 } cypher_update_item;
 
 /* grammar node for typecasts */