[incubator-age] branch master updated: Fix local cached contexts for static procedures. Issue #220

[jg...@apache.org]

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 12edd3e  Fix local cached contexts for static procedures. Issue #220
12edd3e is described below

commit 12edd3e5b5125fc685d84346916062042404a9c0
Author: John Gemignani <jr...@gmail.com>
AuthorDate: Wed May 25 12:34:49 2022 -0700

    Fix local cached contexts for static procedures. Issue #220
    
    This fix allows locally cached contexts to be rebuilt for actions that
    update the specified underlying graph.
    
    Local contexts are tied to their VLE grammar node IDs, which is unique to
    each command. This ID allows the caching of contexts, which allows their
    reuse, thus saving the expensive costs of rebuilding a new local context
    every time, for the same command. Its expected use was within a single
    command line. This is because the VLE SRF may get invoked many times
    through the single grammar node's lifetime on that line.
    
    However, it was overlooked in static procedures (stored procedures and
    user created functions) that these grammar nodes might persist longer
    than the expected life of just one command line.
    
    This fix allows them to persist and update along with the graph.
    
    Added regression tests.
---
 regress/expected/cypher_vle.out          | 145 +++++++++++++++++++++++++++++++
 regress/sql/cypher_vle.sql               |  75 ++++++++++++++++
 src/backend/utils/adt/age_global_graph.c |  25 +++++-
 src/backend/utils/adt/age_vle.c          |  11 +++
 src/include/utils/age_global_graph.h     |   1 +
 5 files changed, 254 insertions(+), 3 deletions(-)

diff --git a/regress/expected/cypher_vle.out b/regress/expected/cypher_vle.out
index 5ae8449..5f8f922 100644
--- a/regress/expected/cypher_vle.out
+++ b/regress/expected/cypher_vle.out
@@ -616,6 +616,151 @@ NOTICE:  graph "mygraph" has been dropped
 (1 row)
 
 COMMIT;
+--
+-- Test VLE inside procedures
+--
+SELECT create_graph('mygraph');
+NOTICE:  graph "mygraph" has been created
+ create_graph 
+--------------
+ 
+(1 row)
+
+SELECT create_vlabel('mygraph', 'head');
+NOTICE:  VLabel "head" has been created
+ create_vlabel 
+---------------
+ 
+(1 row)
+
+SELECT create_vlabel('mygraph', 'tail');
+NOTICE:  VLabel "tail" has been created
+ create_vlabel 
+---------------
+ 
+(1 row)
+
+SELECT create_vlabel('mygraph', 'node');
+NOTICE:  VLabel "node" has been created
+ create_vlabel 
+---------------
+ 
+(1 row)
+
+SELECT create_elabel('mygraph', 'next');
+NOTICE:  ELabel "next" has been created
+ create_elabel 
+---------------
+ 
+(1 row)
+
+CREATE OR REPLACE FUNCTION create_list(list_name text)
+RETURNS void
+LANGUAGE 'plpgsql'
+AS $$
+DECLARE
+    ag_param agtype;
+BEGIN
+    ag_param = FORMAT('{"list_name": "%s"}', $1)::agtype;
+    PERFORM * FROM cypher('mygraph', $CYPHER$
+        MERGE (:head {name: $list_name})-[:next]->(:tail {name: $list_name})
+    $CYPHER$, ag_param) AS (a agtype);
+END $$;
+CREATE OR REPLACE FUNCTION prepend_node(list_name text, node_content text)
+RETURNS void
+LANGUAGE 'plpgsql'
+AS $$
+DECLARE
+    ag_param agtype;
+BEGIN
+    ag_param = FORMAT('{"list_name": "%s", "node_content": "%s"}', $1, $2)::agtype;
+    PERFORM * FROM cypher('mygraph', $CYPHER$
+        MATCH (h:head {name: $list_name})-[e:next]->(v)
+        DELETE e
+        CREATE (h)-[:next]->(:node {content: $node_content})-[:next]->(v)
+    $CYPHER$, ag_param) AS (a agtype);
+END $$;
+CREATE OR REPLACE FUNCTION show_list_use_vle(list_name text)
+RETURNS TABLE(node agtype)
+LANGUAGE 'plpgsql'
+AS $$
+DECLARE
+    ag_param agtype;
+BEGIN
+    ag_param = FORMAT('{"list_name": "%s"}', $1)::agtype;
+    RETURN QUERY
+    SELECT * FROM cypher('mygraph', $CYPHER$
+        MATCH (h:head {name: $list_name})-[e:next*]->(v:node)
+        RETURN v
+    $CYPHER$, ag_param) AS (node agtype);
+END $$;
+-- create a list
+SELECT create_list('list01');
+ create_list 
+-------------
+ 
+(1 row)
+
+-- prepend a node 'a'
+-- should find 1 row
+SELECT prepend_node('list01', 'a');
+ prepend_node 
+--------------
+ 
+(1 row)
+
+SELECT * FROM show_list_use_vle('list01');
+                                       node                                        
+-----------------------------------------------------------------------------------
+ {"id": 1407374883553281, "label": "node", "properties": {"content": "a"}}::vertex
+(1 row)
+
+-- prepend a node 'b'
+-- should find 2 rows
+SELECT prepend_node('list01', 'b');
+ prepend_node 
+--------------
+ 
+(1 row)
+
+SELECT * FROM show_list_use_vle('list01');
+                                       node                                        
+-----------------------------------------------------------------------------------
+ {"id": 1407374883553281, "label": "node", "properties": {"content": "a"}}::vertex
+ {"id": 1407374883553282, "label": "node", "properties": {"content": "b"}}::vertex
+(2 rows)
+
+-- prepend a node 'c'
+-- should find 3 rows
+SELECT prepend_node('list01', 'c');
+ prepend_node 
+--------------
+ 
+(1 row)
+
+SELECT * FROM show_list_use_vle('list01');
+                                       node                                        
+-----------------------------------------------------------------------------------
+ {"id": 1407374883553281, "label": "node", "properties": {"content": "a"}}::vertex
+ {"id": 1407374883553282, "label": "node", "properties": {"content": "b"}}::vertex
+ {"id": 1407374883553283, "label": "node", "properties": {"content": "c"}}::vertex
+(3 rows)
+
+DROP FUNCTION show_list_use_vle;
+SELECT drop_graph('mygraph', true);
+NOTICE:  drop cascades to 6 other objects
+DETAIL:  drop cascades to table mygraph._ag_label_vertex
+drop cascades to table mygraph._ag_label_edge
+drop cascades to table mygraph.head
+drop cascades to table mygraph.tail
+drop cascades to table mygraph.node
+drop cascades to table mygraph.next
+NOTICE:  graph "mygraph" has been dropped
+ drop_graph 
+------------
+ 
+(1 row)
+
 --
 -- Clean up
 --
diff --git a/regress/sql/cypher_vle.sql b/regress/sql/cypher_vle.sql
index 193d30b..468135e 100644
--- a/regress/sql/cypher_vle.sql
+++ b/regress/sql/cypher_vle.sql
@@ -215,6 +215,81 @@ SELECT drop_graph('mygraph', true);
 
 COMMIT;
 
+--
+-- Test VLE inside procedures
+--
+
+SELECT create_graph('mygraph');
+SELECT create_vlabel('mygraph', 'head');
+SELECT create_vlabel('mygraph', 'tail');
+SELECT create_vlabel('mygraph', 'node');
+SELECT create_elabel('mygraph', 'next');
+
+CREATE OR REPLACE FUNCTION create_list(list_name text)
+RETURNS void
+LANGUAGE 'plpgsql'
+AS $$
+DECLARE
+    ag_param agtype;
+BEGIN
+    ag_param = FORMAT('{"list_name": "%s"}', $1)::agtype;
+    PERFORM * FROM cypher('mygraph', $CYPHER$
+        MERGE (:head {name: $list_name})-[:next]->(:tail {name: $list_name})
+    $CYPHER$, ag_param) AS (a agtype);
+END $$;
+
+CREATE OR REPLACE FUNCTION prepend_node(list_name text, node_content text)
+RETURNS void
+LANGUAGE 'plpgsql'
+AS $$
+DECLARE
+    ag_param agtype;
+BEGIN
+    ag_param = FORMAT('{"list_name": "%s", "node_content": "%s"}', $1, $2)::agtype;
+    PERFORM * FROM cypher('mygraph', $CYPHER$
+        MATCH (h:head {name: $list_name})-[e:next]->(v)
+        DELETE e
+        CREATE (h)-[:next]->(:node {content: $node_content})-[:next]->(v)
+    $CYPHER$, ag_param) AS (a agtype);
+END $$;
+
+CREATE OR REPLACE FUNCTION show_list_use_vle(list_name text)
+RETURNS TABLE(node agtype)
+LANGUAGE 'plpgsql'
+AS $$
+DECLARE
+    ag_param agtype;
+BEGIN
+    ag_param = FORMAT('{"list_name": "%s"}', $1)::agtype;
+    RETURN QUERY
+    SELECT * FROM cypher('mygraph', $CYPHER$
+        MATCH (h:head {name: $list_name})-[e:next*]->(v:node)
+        RETURN v
+    $CYPHER$, ag_param) AS (node agtype);
+END $$;
+
+-- create a list
+SELECT create_list('list01');
+
+-- prepend a node 'a'
+-- should find 1 row
+SELECT prepend_node('list01', 'a');
+SELECT * FROM show_list_use_vle('list01');
+
+-- prepend a node 'b'
+-- should find 2 rows
+SELECT prepend_node('list01', 'b');
+SELECT * FROM show_list_use_vle('list01');
+
+-- prepend a node 'c'
+-- should find 3 rows
+SELECT prepend_node('list01', 'c');
+SELECT * FROM show_list_use_vle('list01');
+
+DROP FUNCTION show_list_use_vle;
+
+SELECT drop_graph('mygraph', true);
+
 --
 -- Clean up
 --
diff --git a/src/backend/utils/adt/age_global_graph.c b/src/backend/utils/adt/age_global_graph.c
index ec01882..3da11b4 100644
--- a/src/backend/utils/adt/age_global_graph.c
+++ b/src/backend/utils/adt/age_global_graph.c
@@ -107,6 +107,27 @@ static bool insert_vertex_entry(GRAPH_global_context *ggctx, graphid vertex_id,
                                 Oid vertex_label_table_oid,
                                 Datum vertex_properties);
 /* definitions */
+
+/*
+ * Helper function to determine validity of the passed GRAPH_global_context.
+ * This is based off of the current active snaphot, to see if the graph could
+ * have been modified. Ideally, we should find a way to more accurately know
+ * whether the particular graph was modified.
+ */
+bool is_ggctx_invalid(GRAPH_global_context *ggctx)
+{
+    Snapshot snap = GetActiveSnapshot();
+
+    /*
+     * If the transaction ids (xmin or xmax) or currentCommandId (curcid) have
+     * changed, then we have a graph that was updated. This means that the
+     * global context for this graph is no longer valid.
+     */
+    return (ggctx->xmin != snap->xmin ||
+            ggctx->xmax != snap->xmax ||
+            ggctx->curcid != snap->curcid);
+
+}
 /*
  * Helper function to create the global vertex and edge hashtables. One
  * hashtable will hold the vertex, its edges (both incoming and exiting) as a
@@ -676,9 +697,7 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name,
         GRAPH_global_context *next_ggctx = curr_ggctx->next;
 
         /* if the transaction ids have changed, we have an invalid graph */
-        if (curr_ggctx->xmin != GetActiveSnapshot()->xmin ||
-            curr_ggctx->xmax != GetActiveSnapshot()->xmax ||
-            curr_ggctx->curcid != GetActiveSnapshot()->curcid)
+        if (is_ggctx_invalid(curr_ggctx))
         {
             /*
              * If prev_ggctx is NULL then we are freeing the top of the
diff --git a/src/backend/utils/adt/age_vle.c b/src/backend/utils/adt/age_vle.c
index 20f2cae..30c7397 100644
--- a/src/backend/utils/adt/age_vle.c
+++ b/src/backend/utils/adt/age_vle.c
@@ -200,6 +200,17 @@ static VLE_local_context *get_cached_VLE_local_context(int64 vle_grammar_node_id
                  * If ggctx != vlelctx->ggctx, then vlelctx needs to be updated.
                  * In the end, vlelctx->ggctx will be set to ggctx.
                  */
+
+                /*
+                 * If the returned ggctx isn't valid (there was some update to
+                 * the underlying graph), then set it to NULL. This will force a
+                 * rebuild of it.
+                 */
+                if (ggctx != NULL && is_ggctx_invalid(ggctx))
+                {
+                    ggctx = NULL;
+                }
+
                 vlelctx->ggctx = ggctx;
 
                 /*
diff --git a/src/include/utils/age_global_graph.h b/src/include/utils/age_global_graph.h
index ad6cd59..daa1e51 100644
--- a/src/include/utils/age_global_graph.h
+++ b/src/include/utils/age_global_graph.h
@@ -41,6 +41,7 @@ typedef struct GRAPH_global_context GRAPH_global_context;
 GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name,
                                                    Oid graph_oid);
 GRAPH_global_context *find_GRAPH_global_context(Oid graph_oid);
+bool is_ggctx_invalid(GRAPH_global_context *ggctx);
 /* GRAPH retrieval functions */
 ListGraphId *get_graph_vertices(GRAPH_global_context *ggctx);
 vertex_entry *get_vertex_entry(GRAPH_global_context *ggctx,