From 176f0cc0713ac9cd37e0eb5e77a5c769c546ee22 Mon Sep 17 00:00:00 2001 From: John Gemignani Date: Wed, 6 Mar 2024 00:33:51 -0800 Subject: [PATCH] Fix Issue 1634: Setting all properties with map object causes error (#1637) Fixed the issue where setting the properties would not work, if it where an object passed as the properties. Added regression tests. --- regress/expected/cypher_set.out | 66 +++++++++++++++++++++++++++++++++ regress/sql/cypher_set.sql | 29 ++++++++++++++- src/backend/utils/adt/agtype.c | 27 ++++++++++++-- 3 files changed, 117 insertions(+), 5 deletions(-) diff --git a/regress/expected/cypher_set.out b/regress/expected/cypher_set.out index a2ea2edc1..9a094fcda 100644 --- a/regress/expected/cypher_set.out +++ b/regress/expected/cypher_set.out @@ -872,6 +872,59 @@ $$) AS (p agtype); {"id": 281474976710658, "label": "", "properties": {"n0": true, "n1": false, "n2": true}}::vertex (1 row) +-- +-- Issue 1634: Setting all properties with map object causes error +-- +SELECT * FROM create_graph('issue_1634'); +NOTICE: graph "issue_1634" has been created + create_graph +-------------- + +(1 row) + +-- this did not work and was fixed +SELECT * FROM cypher('issue_1634', $$ WITH {first: 'jon', last: 'snow'} AS map + MERGE (v:PERSION {id: '1'}) + SET v=map + RETURN v,map $$) as (v agtype, map agtype); + v | map +-----------------------------------------------------------------------------------------------------+---------------------------------- + {"id": 844424930131969, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex | {"last": "snow", "first": "jon"} +(1 row) + +-- these 2 did work and are added as extra tests +SELECT * FROM cypher('issue_1634', $$ MATCH (u) DELETE (u) $$) AS (u agtype); + u +--- +(0 rows) + +SELECT * FROM cypher('issue_1634', $$ WITH {first: 'jon', last: 'snow'} AS map + MERGE (v:PERSION {id: '1'}) + SET v.first=map.first, v.last=map.last + RETURN v,map $$) as (v agtype, map agtype); + v | map +----------------------------------------------------------------------------------------------------------------+---------------------------------- + {"id": 844424930131970, "label": "PERSION", "properties": {"id": "1", "last": "snow", "first": "jon"}}::vertex | {"last": "snow", "first": "jon"} +(1 row) + +SELECT * FROM cypher('issue_1634', $$ MATCH (u) DELETE (u) $$) AS (u agtype); + u +--- +(0 rows) + +SELECT * FROM cypher('issue_1634', $$ MERGE (v:PERSION {id: '1'}) + SET v={first: 'jon', last: 'snow'} + RETURN v $$) as (v agtype); + v +----------------------------------------------------------------------------------------------------- + {"id": 844424930131971, "label": "PERSION", "properties": {"last": "snow", "first": "jon"}}::vertex +(1 row) + +SELECT * FROM cypher('issue_1634', $$ MATCH (u) DELETE (u) $$) AS (u agtype); + u +--- +(0 rows) + -- -- Clean up -- @@ -911,4 +964,17 @@ NOTICE: graph "cypher_set_1" has been dropped (1 row) +SELECT drop_graph('issue_1634', true); +NOTICE: drop cascades to 3 other objects +DETAIL: drop cascades to table issue_1634._ag_label_vertex +drop cascades to table issue_1634._ag_label_edge +drop cascades to table issue_1634."PERSION" +NOTICE: graph "issue_1634" has been dropped + drop_graph +------------ + +(1 row) + +-- +-- End -- diff --git a/regress/sql/cypher_set.sql b/regress/sql/cypher_set.sql index 316d59017..e03af8472 100644 --- a/regress/sql/cypher_set.sql +++ b/regress/sql/cypher_set.sql @@ -313,6 +313,31 @@ SELECT * FROM cypher('cypher_set_1', $$ CREATE (x) SET x.n0 = (true OR false), x.n1 = (false AND false), x.n2 = (false = false) RETURN x $$) AS (p agtype); +-- +-- Issue 1634: Setting all properties with map object causes error +-- +SELECT * FROM create_graph('issue_1634'); + +-- this did not work and was fixed +SELECT * FROM cypher('issue_1634', $$ WITH {first: 'jon', last: 'snow'} AS map + MERGE (v:PERSION {id: '1'}) + SET v=map + RETURN v,map $$) as (v agtype, map agtype); + +-- these 2 did work and are added as extra tests +SELECT * FROM cypher('issue_1634', $$ MATCH (u) DELETE (u) $$) AS (u agtype); +SELECT * FROM cypher('issue_1634', $$ WITH {first: 'jon', last: 'snow'} AS map + MERGE (v:PERSION {id: '1'}) + SET v.first=map.first, v.last=map.last + RETURN v,map $$) as (v agtype, map agtype); + +SELECT * FROM cypher('issue_1634', $$ MATCH (u) DELETE (u) $$) AS (u agtype); +SELECT * FROM cypher('issue_1634', $$ MERGE (v:PERSION {id: '1'}) + SET v={first: 'jon', last: 'snow'} + RETURN v $$) as (v agtype); + +SELECT * FROM cypher('issue_1634', $$ MATCH (u) DELETE (u) $$) AS (u agtype); + -- -- Clean up -- @@ -320,6 +345,8 @@ DROP TABLE tbl; DROP FUNCTION set_test; SELECT drop_graph('cypher_set', true); SELECT drop_graph('cypher_set_1', true); +SELECT drop_graph('issue_1634', true); -- - +-- End +-- diff --git a/src/backend/utils/adt/agtype.c b/src/backend/utils/adt/agtype.c index 32983d653..4b3aaaadb 100644 --- a/src/backend/utils/adt/agtype.c +++ b/src/backend/utils/adt/agtype.c @@ -5557,25 +5557,44 @@ Datum age_properties(PG_FUNCTION_ARGS) /* check for null */ if (PG_ARGISNULL(0)) + { PG_RETURN_NULL(); + } agt_arg = AG_GET_ARG_AGTYPE_P(0); - /* check for a scalar object */ - if (!AGT_ROOT_IS_SCALAR(agt_arg)) + /* check for a scalar or regular object */ + + if (!AGT_ROOT_IS_SCALAR(agt_arg) && !AGT_ROOT_IS_OBJECT(agt_arg)) + { ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("properties() argument must resolve to a scalar value"))); + errmsg("properties() argument must resolve to an object"))); + } + + /* + * If it isn't an array (wrapped scalar) and is an object, just return it. + * This is necessary for some cases where an object may be passed in. For + * example, SET v={blah}. + */ + if (!AGT_ROOT_IS_ARRAY(agt_arg) && AGT_ROOT_IS_OBJECT(agt_arg)) + { + PG_RETURN_POINTER(agt_arg); + } /* get the object out of the array */ agtv_object = get_ith_agtype_value_from_container(&agt_arg->root, 0); /* is it an agtype null? */ if (agtv_object->type == AGTV_NULL) - PG_RETURN_NULL(); + { + PG_RETURN_NULL(); + } /* check for proper agtype */ if (agtv_object->type != AGTV_VERTEX && agtv_object->type != AGTV_EDGE) + { ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("properties() argument must be a vertex, an edge or null"))); + } agtv_result = GET_AGTYPE_VALUE_OBJECT_VALUE(agtv_object, "properties");