This is an automated email from the ASF dual-hosted git repository. jgemignani pushed a commit to branch PG15 in repository https://gitbox.apache.org/repos/asf/age.git
commit fb88a7020d7be98e8ff3a41dc596389e0c2b923d Author: Zainab Saad <[email protected]> AuthorDate: Tue Sep 5 20:45:49 2023 +0500 Fix Issue#1159: Server terminates for SET plus-equal (#1160) The previous implementation of alter_properties() in agtype.c while copying the original properties ignored non-scalar value cases, this PR fixes that --- regress/expected/cypher_set.out | 49 ++++++++++++++++- regress/sql/cypher_set.sql | 26 +++++++++ src/backend/utils/adt/agtype.c | 12 +---- src/backend/utils/adt/agtype_util.c | 102 ++++++++++++++++++++++++++++++++++++ src/include/utils/agtype.h | 3 ++ 5 files changed, 181 insertions(+), 11 deletions(-) diff --git a/regress/expected/cypher_set.out b/regress/expected/cypher_set.out index 0310ae01..1b391a0f 100644 --- a/regress/expected/cypher_set.out +++ b/regress/expected/cypher_set.out @@ -787,6 +787,11 @@ SELECT * FROM cypher('cypher_set_1', $$ CREATE (a:Robert {name:'Robert', role:'m --- (0 rows) +SELECT * FROM cypher('cypher_set_1',$$ CREATE (a: VertexA {map: {a: 1, b: {c: 2, d: []}, c: [{d: -100, e: []}]}, list: [1, "string", [{a: []}, [[1, 2]]]], bool: true, num: -1.9::numeric, str: "string"})$$) as (a agtype); + a +--- +(0 rows) + -- test copying properties between entities SELECT * FROM cypher('cypher_set_1', $$ MATCH (at {name: 'Andy'}), (pn {name: 'Peter'}) @@ -869,6 +874,47 @@ $$) AS (p agtype); {"id": 2251799813685249, "label": "Robert", "properties": {"age": 47, "city": "London", "name": "Rob"}}::vertex (1 row) +-- test plus-equal with original properties having non-scalar values +SELECT * FROM cypher('cypher_set_1', $$ + MATCH (p {map: {}}) + SET p += {json: {a: -1, b: ['a', -1, true], c: {d: 'string'}}} + RETURN p +$$) AS (p agtype); + p +-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + {"id": 2533274790395905, "label": "VertexA", "properties": {"map": {"a": 1, "b": {"c": 2, "d": []}, "c": [{"d": -100, "e": []}]}, "num": -1.9::numeric, "str": "string", "bool": true, "json": {"a": -1, "b": ["a", -1, true], "c": {"d": "string"}}, "list": [1, "string", [{"a": []}, [[1, 2]]]]}}::vertex +(1 row) + +SELECT * FROM cypher('cypher_set_1', $$ + MATCH (p: VertexA {map: {}}) + SET p += {list_upd: [1, 2, 3, 4, 5]} + RETURN p +$$) AS (p agtype); + p +------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + {"id": 2533274790395905, "label": "VertexA", "properties": {"map": {"a": 1, "b": {"c": 2, "d": []}, "c": [{"d": -100, "e": []}]}, "num": -1.9::numeric, "str": "string", "bool": true, "json": {"a": -1, "b": ["a", -1, true], "c": {"d": "string"}}, "list": [1, "string", [{"a": []}, [[1, 2]]]], "list_upd": [1, 2, 3, 4, 5]}}::vertex +(1 row) + +SELECT * FROM cypher('cypher_set_1', $$ + MATCH (p: VertexA) + SET p += {vertex: {id: 281474976710659, label: "", properties: {a: 1, b: [1, 2, 3]}}::vertex} + RETURN p +$$) AS (p agtype); + p +------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + {"id": 2533274790395905, "label": "VertexA", "properties": {"map": {"a": 1, "b": {"c": 2, "d": []}, "c": [{"d": -100, "e": []}]}, "num": -1.9::numeric, "str": "string", "bool": true, "json": {"a": -1, "b": ["a", -1, true], "c": {"d": "string"}}, "list": [1, "string", [{"a": []}, [[1, 2]]]], "vertex": {"id": 281474976710659, "label": "", "properties": {"a": 1, "b": [1, 2, 3]}}::vertex, "list_upd": [1, 2, 3, 4, 5]}}::vertex +(1 row) + +SELECT * FROM cypher('cypher_set_1', $$ + MATCH (p {map: {}}) + SET p += {} + RETURN p +$$) AS (p agtype); + p +------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + {"id": 2533274790395905, "label": "VertexA", "properties": {"map": {"a": 1, "b": {"c": 2, "d": []}, "c": [{"d": -100, "e": []}]}, "num": -1.9::numeric, "str": "string", "bool": true, "json": {"a": -1, "b": ["a", -1, true], "c": {"d": "string"}}, "list": [1, "string", [{"a": []}, [[1, 2]]]], "vertex": {"id": 281474976710659, "label": "", "properties": {"a": 1, "b": [1, 2, 3]}}::vertex, "list_upd": [1, 2, 3, 4, 5]}}::vertex +(1 row) + -- -- Check passing mismatched types with SET -- Issue 899 @@ -912,7 +958,7 @@ NOTICE: graph "cypher_set" has been dropped (1 row) SELECT drop_graph('cypher_set_1', true); -NOTICE: drop cascades to 8 other objects +NOTICE: drop cascades to 9 other objects DETAIL: drop cascades to table cypher_set_1._ag_label_vertex drop cascades to table cypher_set_1._ag_label_edge drop cascades to table cypher_set_1."Andy" @@ -921,6 +967,7 @@ drop cascades to table cypher_set_1."Kevin" drop cascades to table cypher_set_1."Matt" drop cascades to table cypher_set_1."Juan" drop cascades to table cypher_set_1."Robert" +drop cascades to table cypher_set_1."VertexA" NOTICE: graph "cypher_set_1" has been dropped drop_graph ------------ diff --git a/regress/sql/cypher_set.sql b/regress/sql/cypher_set.sql index 484cbff7..6720d6de 100644 --- a/regress/sql/cypher_set.sql +++ b/regress/sql/cypher_set.sql @@ -260,6 +260,7 @@ SELECT * FROM cypher('cypher_set_1', $$ CREATE (a:Kevin {name:'Kevin', age:32, h SELECT * FROM cypher('cypher_set_1', $$ CREATE (a:Matt {name:'Matt', city:'Toronto'}) $$) AS (a agtype); SELECT * FROM cypher('cypher_set_1', $$ CREATE (a:Juan {name:'Juan', role:'admin'}) $$) AS (a agtype); SELECT * FROM cypher('cypher_set_1', $$ CREATE (a:Robert {name:'Robert', role:'manager', city:'London'}) $$) AS (a agtype); +SELECT * FROM cypher('cypher_set_1',$$ CREATE (a: VertexA {map: {a: 1, b: {c: 2, d: []}, c: [{d: -100, e: []}]}, list: [1, "string", [{a: []}, [[1, 2]]]], bool: true, num: -1.9::numeric, str: "string"})$$) as (a agtype); -- test copying properties between entities SELECT * FROM cypher('cypher_set_1', $$ @@ -316,6 +317,31 @@ SELECT * FROM cypher('cypher_set_1', $$ RETURN p $$) AS (p agtype); +-- test plus-equal with original properties having non-scalar values +SELECT * FROM cypher('cypher_set_1', $$ + MATCH (p {map: {}}) + SET p += {json: {a: -1, b: ['a', -1, true], c: {d: 'string'}}} + RETURN p +$$) AS (p agtype); + +SELECT * FROM cypher('cypher_set_1', $$ + MATCH (p: VertexA {map: {}}) + SET p += {list_upd: [1, 2, 3, 4, 5]} + RETURN p +$$) AS (p agtype); + +SELECT * FROM cypher('cypher_set_1', $$ + MATCH (p: VertexA) + SET p += {vertex: {id: 281474976710659, label: "", properties: {a: 1, b: [1, 2, 3]}}::vertex} + RETURN p +$$) AS (p agtype); + +SELECT * FROM cypher('cypher_set_1', $$ + MATCH (p {map: {}}) + SET p += {} + RETURN p +$$) AS (p agtype); + -- -- Check passing mismatched types with SET -- Issue 899 diff --git a/src/backend/utils/adt/agtype.c b/src/backend/utils/adt/agtype.c index ea254016..2d025a8f 100644 --- a/src/backend/utils/adt/agtype.c +++ b/src/backend/utils/adt/agtype.c @@ -9241,22 +9241,14 @@ agtype_value *alter_properties(agtype_value *original_properties, // Copy original properties. if (original_properties) { - int i; - if (original_properties->type != AGTV_OBJECT) { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("a map is expected"))); } - for (i = 0; i < original_properties->val.object.num_pairs; i++) - { - agtype_pair* pair = original_properties->val.object.pairs + i; - parsed_agtype_value = push_agtype_value(&parse_state, WAGT_KEY, - &pair->key); - parsed_agtype_value = push_agtype_value(&parse_state, WAGT_VALUE, - &pair->value); - } + copy_agtype_value(parse_state, original_properties, + &parsed_agtype_value, true); } // Append new properties. diff --git a/src/backend/utils/adt/agtype_util.c b/src/backend/utils/adt/agtype_util.c index 0583bce6..107d2d0d 100644 --- a/src/backend/utils/adt/agtype_util.c +++ b/src/backend/utils/adt/agtype_util.c @@ -2385,3 +2385,105 @@ void pfree_agtype_in_state(agtype_in_state* value) pfree_agtype_value(value->res); free(value->parse_state); } + +/* + * helper function that recursively unpacks the agtype_value to be copied + * and pushes the scalar values into the copied agtype_value. + * this helps skip the serialization part at some places where the original + * properties passed to the function are in agtype_value format and + * converting it to agtype for iteration can be expensive. + * the caller of this function will need to push start and end object tokens + * on its own as this function might be used in places where pushing only start + * object token at top level is required (for example in alter_properties) + */ +void copy_agtype_value(agtype_parse_state* pstate, + agtype_value* original_agtype_value, + agtype_value **copied_agtype_value, bool is_top_level) +{ + int i = 0; + + /* + * guards against stack overflow due to deeply nested agtype_value + */ + check_stack_depth(); + + /* + * directly pass the agtype_value to be pushed into the copied result + * if type is scalar or binary (array or object) as push_agtype_value + * can unpack binary on its own + */ + if (IS_A_AGTYPE_SCALAR(original_agtype_value) || + original_agtype_value->type == AGTV_BINARY) + { + *copied_agtype_value = push_agtype_value(&pstate, WAGT_ELEM, + original_agtype_value); + } + /* + * if the passed in type is object or array, unpack it + * until we are left with a scalar value to push to copied result + */ + else if (original_agtype_value->type == AGTV_OBJECT) + { + if (!is_top_level) + { + *copied_agtype_value = push_agtype_value(&pstate, + WAGT_BEGIN_OBJECT, + NULL); + } + + for (; i < original_agtype_value->val.object.num_pairs; i ++) + { + agtype_pair *pair = original_agtype_value->val.object.pairs + i; + *copied_agtype_value = push_agtype_value(&pstate, WAGT_KEY, + &pair->key); + + if (IS_A_AGTYPE_SCALAR(&pair->value)) + { + *copied_agtype_value = push_agtype_value(&pstate, WAGT_VALUE, + &pair->value); + } + else + { + /* do a recursive call once a non-scalar value is reached */ + copy_agtype_value(pstate, &pair->value, copied_agtype_value, + false); + } + } + + if (!is_top_level) + { + *copied_agtype_value = push_agtype_value(&pstate, WAGT_END_OBJECT, + NULL); + } + } + else if (original_agtype_value->type == AGTV_ARRAY) + { + *copied_agtype_value = push_agtype_value(&pstate, WAGT_BEGIN_ARRAY, + NULL); + + for (; i < original_agtype_value->val.array.num_elems; i++) + { + agtype_value elem = original_agtype_value->val.array.elems[i]; + + if (IS_A_AGTYPE_SCALAR(&elem)) + { + *copied_agtype_value = push_agtype_value(&pstate, WAGT_ELEM, + &elem); + } + else + { + /* do a recursive call once a non-scalar value is reached */ + copy_agtype_value(pstate, &elem, copied_agtype_value, false); + } + } + + *copied_agtype_value = push_agtype_value(&pstate, WAGT_END_ARRAY, + NULL); + } + else + { + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid type provided for copy_agtype_value"))); + } +} \ No newline at end of file diff --git a/src/include/utils/agtype.h b/src/include/utils/agtype.h index 436470ef..7020ac8e 100644 --- a/src/include/utils/agtype.h +++ b/src/include/utils/agtype.h @@ -489,6 +489,9 @@ void convert_extended_object(StringInfo buffer, agtentry *pheader, agtype_value *val); Datum get_numeric_datum_from_agtype_value(agtype_value *agtv); bool is_numeric_result(agtype_value *lhs, agtype_value *rhs); +void copy_agtype_value(agtype_parse_state* pstate, + agtype_value* original_agtype_value, + agtype_value **copied_agtype_value, bool is_top_level); /* agtype.c support functions */ /*
