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.

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

VLE still has a bug when I try to run PL/pgSQL functions with VLE statements inside #220

Closed
jihot2000 opened this issue May 24, 2022 · 22 comments
Labels
documentation Improvements or additions to documentation question Further information is requested

Comments

@jihot2000
Copy link

Describe the bug

VLE still has a bug when I try to run my PL/pgSQL functions with VLE statements inside.

How are you accessing AGE (Command line, driver, etc.)?
run a sql file using psql command line

the sql file

CREATE SCHEMA test_vle;

SET search_path = test_vle, ag_catalog, "$user";

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 OR REPLACE FUNCTION show_list_use_loop(list_name text)
RETURNS TABLE(node agtype)
LANGUAGE 'plpgsql'
AS $$
DECLARE
    node_id bigint;
    ag_param agtype;
BEGIN
    ag_param = FORMAT('{"list_name": "%s"}', $1)::agtype;
    SELECT g.id INTO node_id FROM cypher('mygraph', $CYPHER$
        MATCH (h:head {name: $list_name})
        RETURN id(h)
    $CYPHER$, ag_param) AS g(id bigint);

    IF node_id IS NULL THEN
        RETURN;
    END IF;

    LOOP
        ag_param = FORMAT('{"node_id": %s}', node_id)::agtype;
        SELECT g.id, g.node INTO node_id, node FROM cypher('mygraph', $CYPHER$
            MATCH (p)-[e:next]->(v:node)
            WHERE id(p) = $node_id
            RETURN id(v), v
        $CYPHER$, ag_param) AS g(id bigint, node agtype);
        EXIT WHEN node_id IS NULL;
        RETURN NEXT;
    END LOOP;
END $$;

-- create a list
SELECT create_list('list01');

-- prepend a node 'a'
SELECT prepend_node('list01', 'a');
SELECT * FROM show_list_use_vle('list01');    -- this line shows node 'a' as expected
SELECT * FROM show_list_use_loop('list01');    -- this line show node 'a' as expected

-- prepend a node 'b'
SELECT prepend_node('list01', 'b');
SELECT * FROM show_list_use_vle('list01');    -- bug: this line only shows node 'a'
SELECT * FROM show_list_use_loop('list01');    -- this line shows node 'b' and 'a' as expected

SELECT drop_graph('mygraph', true);

DROP SCHEMA test_vle CASCADE;

Expected behavior

The bug show_list_use_vle line only shows node 'a':

                                       node                                        
-----------------------------------------------------------------------------------
 {"id": 1407374883553281, "label": "node", "properties": {"content": "a"}}::vertex
(1 row)

The show_list_use_loop line shows the expected result:

                                       node                                        
-----------------------------------------------------------------------------------
 {"id": 1407374883553282, "label": "node", "properties": {"content": "b"}}::vertex
 {"id": 1407374883553281, "label": "node", "properties": {"content": "a"}}::vertex
(2 rows)

Environment (please complete the following information):
PostgreSQL 11 and AGE commit id 95ca659

@jihot2000 jihot2000 added the bug Something isn't working label May 24, 2022
@JoshInnis
Copy link
Contributor

Hi, you need to isolate the vle Match ()-[*]-() with cypher write clauses, eg: CREATE, DELETE, SET, etc. They are incompatible at the moment.

@jihot2000
Copy link
Author

Hi, you need to isolate the vle Match ()-[*]-() with cypher write clauses, eg: CREATE, DELETE, SET, etc. They are incompatible at the moment.

@JoshInnis But I only use Match ()-[*]-() for query in the show_list_use_vle function

@JoshInnis
Copy link
Contributor

@jihot2000
Sorry, In your code in the prepend function I see DELETE and CREATE. I will take a closer look

@JoshInnis
Copy link
Contributor

There is a fundamental difference between show_list_use_loop and show_list_use_vle add :head to the variable p in show_list_use_loop and change :node for variable v to :tail . Hope that helps :).

@JoshInnis JoshInnis added the question Further information is requested label May 24, 2022
@jihot2000
Copy link
Author

@JoshInnis I didn't get your point.

It is show_list_use_vle that has a bug, not show_list_use_loop.

@JoshInnis
Copy link
Contributor

show_list_use_loop and show_list_use_vle are not searching for the same thing. There are some subtle differences I would strongly encourage you to explore.

@JoshInnis JoshInnis added documentation Improvements or additions to documentation and removed bug Something isn't working labels May 24, 2022
@jihot2000
Copy link
Author

I think the list structure changed through the script as follows:

after create list:

 (h:head {name: "list01"})-[:next]->(t:tail {name: "list01"})

after prepend node 'a':

 (h:head {name: "list01"})-[:next]->(:node {content: "a"})-[:next]->(t:tail {name: "list01"})

after prepend node 'b':

 (h:head {name: "list01"})-[:next]->(:node {content: "b"})-[:next]->(:node {content: "a"})-[:next]->(t:tail {name: "list01"})

so the cypher statements inside show_list_use_vle

MATCH (h:head {name: "list01"})-[:next*]->(v:node)
RETURN v

should return both 'b' and 'a'.

Do I have some troubles with the analysis above?

@JoshInnis
Copy link
Contributor

JoshInnis commented May 24, 2022

show_list_use_loop doesn't care about :head OR perhaps I am wrong. Forgive me :). How is p mapped to :head?

Or perhaps I should ask what ag_param is doing after LOOP in show_list_use_loop?

@jihot2000
Copy link
Author

show_list_use_loop doesn't care about :head OR perhaps I am wrong. Forgive me :). How is p mapped to :head?

Or perhaps I should ask what ag_param is doing after LOOP in show_list_use_loop?

@JoshInnis

I think the id in the graph is global. The WHERE id(p) = $node_id can get the head node without :head label explicitly.

@JoshInnis
Copy link
Contributor

JoshInnis commented May 24, 2022

??? Graphid AT THIS POINT is guaranteed to be unique on a schema per schema level. We have lost each other, I need more details to assist :). A graph === a schema AT THIS POINT. The WHERE id(p) = $node_id I could give you a 99% guarantee that you are right, but I need details to assist :). global... you have to define this word uniquely.

@JoshInnis
Copy link
Contributor

Please review my first post after the apology for missing your point. I do believe there is a subtle difference between expected and results, where you are wrong and need to review.

@jihot2000
Copy link
Author

jihot2000 commented May 24, 2022

@JoshInnis Thank you for your patience.

Perhaps I have some mistakes. I said "I think the id in the graph is global" meas the id of a vertext inside a graph is globally unique (Maybe this is wrong). For example, the ids of a vertex with label "label1" and a vertext with label "label2" will never be the same. So I can query the two vertices using WHERE id(v) = $its_id without their labels.

You said specify p with :head and replace v:node with v:tail in show_list_use_loop. But that will certainly not get the right result. Because I want to get the nodes between head and tail, not the head or the tail.

@JoshInnis
Copy link
Contributor

@jihot2000 I will not do your job for you. The specifics should help you...

@jihot2000
Copy link
Author

@jihot2000 I will not do your job for you. The specifics should help you...

Thank you. I've read all the docs of AGE. Perhaps I missed something important.

I simplified my sql code into a test case as a sql file above. It is about create/add/show operations of a link list. I don't know how to make it more simple.

@jihot2000
Copy link
Author

jihot2000 commented May 24, 2022

I split the sql file into two files.

First, the use_loop.sql

CREATE SCHEMA test_vle;

SET search_path = test_vle, ag_catalog, "$user";

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_loop(list_name text)
RETURNS TABLE(node agtype)
LANGUAGE 'plpgsql'
AS $$
DECLARE
    node_id bigint;
    ag_param agtype;
BEGIN
    ag_param = FORMAT('{"list_name": "%s"}', $1)::agtype;
    SELECT g.id INTO node_id FROM cypher('mygraph', $CYPHER$
        MATCH (h:head {name: $list_name})
        RETURN id(h)
    $CYPHER$, ag_param) AS g(id bigint);

    IF node_id IS NULL THEN
        RETURN;
    END IF;

    LOOP
        ag_param = FORMAT('{"node_id": %s}', node_id)::agtype;
        SELECT g.id, g.node INTO node_id, node FROM cypher('mygraph', $CYPHER$
            MATCH (p)-[e:next]->(v:node)
            WHERE id(p) = $node_id
            RETURN id(v), v
        $CYPHER$, ag_param) AS g(id bigint, node agtype);
        EXIT WHEN node_id IS NULL;
        RETURN NEXT;
    END LOOP;
END $$;

-- create a list
SELECT create_list('list01');

-- prepend a node 'a'
SELECT prepend_node('list01', 'a');
SELECT * FROM show_list_use_loop('list01');

-- prepend a node 'b'
SELECT prepend_node('list01', 'b');
SELECT * FROM show_list_use_loop('list01');

SELECT drop_graph('mygraph', true);

Its output is right:

CREATE SCHEMA
SET
psql:06.sql:5: NOTICE:  graph "mygraph" has been created
 create_graph 
--------------
 
(1 row)

psql:06.sql:6: NOTICE:  VLabel "head" has been created
 create_vlabel 
---------------
 
(1 row)

psql:06.sql:7: NOTICE:  VLabel "tail" has been created
 create_vlabel 
---------------
 
(1 row)

psql:06.sql:8: NOTICE:  VLabel "node" has been created
 create_vlabel 
---------------
 
(1 row)

psql:06.sql:9: NOTICE:  ELabel "next" has been created
 create_elabel 
---------------
 
(1 row)

CREATE FUNCTION
CREATE FUNCTION
CREATE FUNCTION
 create_list 
-------------
 
(1 row)

 prepend_node 
--------------
 
(1 row)

                                       node                                        
-----------------------------------------------------------------------------------
 {"id": 1407374883553281, "label": "node", "properties": {"content": "a"}}::vertex
(1 row)

 prepend_node 
--------------
 
(1 row)

                                       node                                        
-----------------------------------------------------------------------------------
 {"id": 1407374883553282, "label": "node", "properties": {"content": "b"}}::vertex
 {"id": 1407374883553281, "label": "node", "properties": {"content": "a"}}::vertex
(2 rows)

psql:06.sql:80: 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
psql:06.sql:80: NOTICE:  graph "mygraph" has been dropped
 drop_graph 
------------
 
(1 row)

psql:06.sql:82: NOTICE:  drop cascades to 3 other objects
DETAIL:  drop cascades to function create_list(text)
drop cascades to function prepend_node(text,text)
drop cascades to function show_list_use_loop(text)
DROP SCHEMA

Second, the 'use_vle.sql':

CREATE SCHEMA test_vle;

SET search_path = test_vle, ag_catalog, "$user";

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'
SELECT prepend_node('list01', 'a');
SELECT * FROM show_list_use_vle('list01');

-- prepend a node 'b'
SELECT prepend_node('list01', 'b');
SELECT * FROM show_list_use_vle('list01'); 

SELECT drop_graph('mygraph', true);

DROP SCHEMA test_vle CASCADE;

Its output is wrong:

CREATE SCHEMA
SET
psql:07.sql:5: NOTICE:  graph "mygraph" has been created
 create_graph 
--------------
 
(1 row)

psql:07.sql:6: NOTICE:  VLabel "head" has been created
 create_vlabel 
---------------
 
(1 row)

psql:07.sql:7: NOTICE:  VLabel "tail" has been created
 create_vlabel 
---------------
 
(1 row)

psql:07.sql:8: NOTICE:  VLabel "node" has been created
 create_vlabel 
---------------
 
(1 row)

psql:07.sql:9: NOTICE:  ELabel "next" has been created
 create_elabel 
---------------
 
(1 row)

CREATE FUNCTION
CREATE FUNCTION
CREATE FUNCTION
 create_list 
-------------
 
(1 row)

 prepend_node 
--------------
 
(1 row)

                                       node                                        
-----------------------------------------------------------------------------------
 {"id": 1407374883553281, "label": "node", "properties": {"content": "a"}}::vertex
(1 row)

 prepend_node 
--------------
 
(1 row)

                                       node                                        
-----------------------------------------------------------------------------------
 {"id": 1407374883553281, "label": "node", "properties": {"content": "a"}}::vertex
(1 row)

psql:07.sql:65: 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
psql:07.sql:65: NOTICE:  graph "mygraph" has been dropped
 drop_graph 
------------
 
(1 row)

psql:07.sql:67: NOTICE:  drop cascades to 3 other objects
DETAIL:  drop cascades to function create_list(text)
drop cascades to function prepend_node(text,text)
drop cascades to function show_list_use_vle(text)
DROP SCHEMA

The question is the output after prepend node 'b' of use_loop.sql is right. Both node 'b' and node 'a' are printed. But the output after prepend node 'b' of use_vle.sql is wrong. It only printed node 'a'.

Here is all the information about the question. I hope I am not asking a stupid question :-P

@jihot2000
Copy link
Author

I rewrite my question in issue #222 to make it more clear

@jihot2000
Copy link
Author

jihot2000 commented May 24, 2022

@jihot2000 I will not do your job for you. The specifics should help you...

@JoshInnis Thank you. I reviewd the docs of AGE.

In /intro/types.md, I find

GraphId
Simple entities are assigned a unique graphid. A graphid is a unique composition of the entity's label id and a unique sequence assigned to each label. Note that there will be overlap in ids when comparing entities from different graphs.

So, I made an assumption that the ids of vertices in different labels will not be the same. And I can query a vertex with its id only. If I went wrong with the doc, please let me know.

I rewrite my question in issue #222 . I removed all the functions except show_list_use_vle to make it more clear.

English is not my native language. If some of my words look offensive, I didn't mean to that. Forgive me.

@jrgemignani
Copy link
Contributor

jrgemignani commented May 24, 2022

I have reproduced and found what the issue is with this particular bug. It is not related to the other VLE bug, other than also being an issue with transaction ids. So, if you are happy with that issue being resolved, please close that ticket.

The issue here has to do with the way the VLE caches contexts, which are expensive to recreate. The VLE stores the grammar node ID as an index into a cache (that holds these contexts) for later retrieval for chained non-updating commands with a VLE component.

Grammar node IDs change from position to position, line to line. In other words, they are unique to that particular VLE command. The problem here is that the grammar node does not change as it is in a procedure. So, the VLE caching mechanism sees it as repeating a previous command.

When writing the caching mechanism, I must have overlooked the possibility of the a VLE component inside a procedure where the grammar node ID would be static. I need to think about how this can be resolved and what the implications would be.

@jihot2000
Copy link
Author

@jrgemignani
I have not much about the inner mechanism of AGE. I just feel it is efficient and cool to use VLE. But obviously, I am using it in an unusual way. Most people would use AGE with a language binding driver and use cypher statements like Neo4J. I prefer wrapping cypher statements in a PL/pgSQL function to be like a table. Then I can integrate graphs and tables together.

If it is not a severe bug to be solved, feel free to close this issue.

@jrgemignani
Copy link
Contributor

I have created a patch, that is currently in review, that should address this particular issue. It should be ready by the end of the week.

jrgemignani added a commit that referenced this issue May 26, 2022
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.
@jrgemignani
Copy link
Contributor

jrgemignani commented May 26, 2022

The patch is in and should resolve this issue. Please make sure to close any open tickets related to this issue if it is resolved.

@jihot2000
Copy link
Author

@jrgemignani
Thank you. This patch resolved my issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants