From 80fc2bfecf4529813cb7414e206d151d435eb17b Mon Sep 17 00:00:00 2001 From: John Gemignani Date: Fri, 14 Jul 2023 13:46:53 -0700 Subject: [PATCH] Fix issue 1033: access operator on vle edge lists (#1037) This patch fixes an issue where the edge lists generated by the vle function couldn't be resolved by the agtype_access_operator. Added regression tests. A big thanks to Taha for showing that this function could be simplified. --- regress/expected/cypher_vle.out | 186 ++++++++++++++++++++++++++++++++ regress/sql/cypher_vle.sql | 25 +++++ src/backend/utils/adt/agtype.c | 127 +++++++++++----------- 3 files changed, 278 insertions(+), 60 deletions(-) diff --git a/regress/expected/cypher_vle.out b/regress/expected/cypher_vle.out index e153ce972..71f520dea 100644 --- a/regress/expected/cypher_vle.out +++ b/regress/expected/cypher_vle.out @@ -814,6 +814,192 @@ SELECT * FROM cypher('cypher_vle', $$ MATCH ()-[p *]-() MATCH p=() RETURN p $$)a ERROR: variable "p" already exists LINE 1: ...M cypher('cypher_vle', $$ MATCH ()-[p *]-() MATCH p=() RETUR... ^ +-- issue 1033, agtype_access_operator not working on containerized edges +SELECT create_graph('access'); +NOTICE: graph "access" has been created + create_graph +-------------- + +(1 row) + +SELECT * FROM cypher('access',$$ CREATE ()-[:knows]->() $$) as (results agtype); + results +--------- +(0 rows) + +SELECT * FROM cypher('access',$$ CREATE ()-[:knows]->()-[:knows]->()$$) as (results agtype); + results +--------- +(0 rows) + +SELECT * FROM cypher('access',$$ CREATE ()-[:knows {id:0}]->()-[:knows {id: 1}]->() $$) as (results agtype); + results +--------- +(0 rows) + +SELECT * FROM cypher('access',$$ CREATE ()-[:knows {id:2, arry:[0,1,2,3,{name: "joe"}]}]->()-[:knows {id: 3, arry:[1,3,{name:"john", stats: {age: 1000}}]}]->() $$) as (results agtype); + results +--------- +(0 rows) + +SELECT * FROM cypher('access', $$ MATCH (u)-[e*]->(v) RETURN e $$)as (edges agtype); + edges +----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + [{"id": 844424930131969, "label": "knows", "end_id": 281474976710658, "start_id": 281474976710657, "properties": {}}::edge] + [{"id": 844424930131971, "label": "knows", "end_id": 281474976710660, "start_id": 281474976710659, "properties": {}}::edge] + [{"id": 844424930131971, "label": "knows", "end_id": 281474976710660, "start_id": 281474976710659, "properties": {}}::edge, {"id": 844424930131970, "label": "knows", "end_id": 281474976710661, "start_id": 281474976710660, "properties": {}}::edge] + [{"id": 844424930131970, "label": "knows", "end_id": 281474976710661, "start_id": 281474976710660, "properties": {}}::edge] + [{"id": 844424930131973, "label": "knows", "end_id": 281474976710663, "start_id": 281474976710662, "properties": {"id": 0}}::edge] + [{"id": 844424930131973, "label": "knows", "end_id": 281474976710663, "start_id": 281474976710662, "properties": {"id": 0}}::edge, {"id": 844424930131972, "label": "knows", "end_id": 281474976710664, "start_id": 281474976710663, "properties": {"id": 1}}::edge] + [{"id": 844424930131972, "label": "knows", "end_id": 281474976710664, "start_id": 281474976710663, "properties": {"id": 1}}::edge] + [{"id": 844424930131975, "label": "knows", "end_id": 281474976710666, "start_id": 281474976710665, "properties": {"id": 2, "arry": [0, 1, 2, 3, {"name": "joe"}]}}::edge] + [{"id": 844424930131975, "label": "knows", "end_id": 281474976710666, "start_id": 281474976710665, "properties": {"id": 2, "arry": [0, 1, 2, 3, {"name": "joe"}]}}::edge, {"id": 844424930131974, "label": "knows", "end_id": 281474976710667, "start_id": 281474976710666, "properties": {"id": 3, "arry": [1, 3, {"name": "john", "stats": {"age": 1000}}]}}::edge] + [{"id": 844424930131974, "label": "knows", "end_id": 281474976710667, "start_id": 281474976710666, "properties": {"id": 3, "arry": [1, 3, {"name": "john", "stats": {"age": 1000}}]}}::edge] +(10 rows) + +SELECT * FROM cypher('access', $$ MATCH (u)-[e*2..2]->(v) RETURN e $$)as (edges agtype); + edges +----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + [{"id": 844424930131971, "label": "knows", "end_id": 281474976710660, "start_id": 281474976710659, "properties": {}}::edge, {"id": 844424930131970, "label": "knows", "end_id": 281474976710661, "start_id": 281474976710660, "properties": {}}::edge] + [{"id": 844424930131973, "label": "knows", "end_id": 281474976710663, "start_id": 281474976710662, "properties": {"id": 0}}::edge, {"id": 844424930131972, "label": "knows", "end_id": 281474976710664, "start_id": 281474976710663, "properties": {"id": 1}}::edge] + [{"id": 844424930131975, "label": "knows", "end_id": 281474976710666, "start_id": 281474976710665, "properties": {"id": 2, "arry": [0, 1, 2, 3, {"name": "joe"}]}}::edge, {"id": 844424930131974, "label": "knows", "end_id": 281474976710667, "start_id": 281474976710666, "properties": {"id": 3, "arry": [1, 3, {"name": "john", "stats": {"age": 1000}}]}}::edge] +(3 rows) + +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN properties(e[0]) $$) as (prop_first_edge agtype); + prop_first_edge +-------------------------------------------------- + {} + {"id": 0} + {"id": 2, "arry": [0, 1, 2, 3, {"name": "joe"}]} +(3 rows) + +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN e[0].id $$) as (results agtype); + results +--------- + + 0 + 2 +(3 rows) + +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN e[0].arry[2] $$) as (results agtype); + results +--------- + + + 2 +(3 rows) + +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN properties(e[1]) $$) as (prop_second_edge agtype); + prop_second_edge +--------------------------------------------------------------------- + {} + {"id": 1} + {"id": 3, "arry": [1, 3, {"name": "john", "stats": {"age": 1000}}]} +(3 rows) + +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN e[1].id $$) as (results agtype); + results +--------- + + 1 + 3 +(3 rows) + +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN e[1].arry[2] $$) as (results agtype); + results +------------------------------------------ + + + {"name": "john", "stats": {"age": 1000}} +(3 rows) + +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN e[1].arry[2].stats $$) as (results agtype); + results +--------------- + + + {"age": 1000} +(3 rows) + +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN properties(e[2]) $$) as (prop_third_edge agtype); + prop_third_edge +----------------- + + + +(3 rows) + +SELECT * FROM cypher('access',$$ MATCH ()-[e*]->() RETURN properties(e[0]), properties(e[1]) $$) as (prop_1st agtype, prop_2nd agtype); + prop_1st | prop_2nd +---------------------------------------------------------------------+--------------------------------------------------------------------- + {} | + {} | + {} | {} + {} | + {"id": 0} | + {"id": 0} | {"id": 1} + {"id": 1} | + {"id": 2, "arry": [0, 1, 2, 3, {"name": "joe"}]} | + {"id": 2, "arry": [0, 1, 2, 3, {"name": "joe"}]} | {"id": 3, "arry": [1, 3, {"name": "john", "stats": {"age": 1000}}]} + {"id": 3, "arry": [1, 3, {"name": "john", "stats": {"age": 1000}}]} | +(10 rows) + +SELECT * FROM cypher('access',$$ MATCH ()-[e*]->() RETURN e[0].id, e[1].id $$) as (results_1st agtype, results_2nd agtype); + results_1st | results_2nd +-------------+------------- + | + | + | + | + 0 | + 0 | 1 + 1 | + 2 | + 2 | 3 + 3 | +(10 rows) + +SELECT * FROM cypher('access',$$ MATCH ()-[e*]->() RETURN e[0].arry, e[1].arry $$) as (results_1st agtype, results_2nd agtype); + results_1st | results_2nd +--------------------------------------------------+-------------------------------------------------- + | + | + | + | + | + | + | + [0, 1, 2, 3, {"name": "joe"}] | + [0, 1, 2, 3, {"name": "joe"}] | [1, 3, {"name": "john", "stats": {"age": 1000}}] + [1, 3, {"name": "john", "stats": {"age": 1000}}] | +(10 rows) + +SELECT * FROM cypher('access',$$ MATCH ()-[e*]->() RETURN e[0].arry[2], e[1].arry[2] $$) as (results_1st agtype, results_2nd agtype); + results_1st | results_2nd +------------------------------------------+------------------------------------------ + | + | + | + | + | + | + | + 2 | + 2 | {"name": "john", "stats": {"age": 1000}} + {"name": "john", "stats": {"age": 1000}} | +(10 rows) + +SELECT drop_graph('access', true); +NOTICE: drop cascades to 3 other objects +DETAIL: drop cascades to table access._ag_label_vertex +drop cascades to table access._ag_label_edge +drop cascades to table access.knows +NOTICE: graph "access" has been dropped + drop_graph +------------ + +(1 row) + -- -- Clean up -- diff --git a/regress/sql/cypher_vle.sql b/regress/sql/cypher_vle.sql index 13cf05944..33cf11da7 100644 --- a/regress/sql/cypher_vle.sql +++ b/regress/sql/cypher_vle.sql @@ -305,6 +305,31 @@ SELECT * FROM cypher('cypher_vle', $$ MATCH ()-[p *]-() MATCH (p) RETURN p $$)as SELECT * FROM cypher('cypher_vle', $$ MATCH p=() MATCH ()-[p *]-() RETURN p $$)as (p agtype); SELECT * FROM cypher('cypher_vle', $$ MATCH ()-[p *]-() MATCH p=() RETURN p $$)as (p agtype); +-- issue 1033, agtype_access_operator not working on containerized edges +SELECT create_graph('access'); + +SELECT * FROM cypher('access',$$ CREATE ()-[:knows]->() $$) as (results agtype); +SELECT * FROM cypher('access',$$ CREATE ()-[:knows]->()-[:knows]->()$$) as (results agtype); +SELECT * FROM cypher('access',$$ CREATE ()-[:knows {id:0}]->()-[:knows {id: 1}]->() $$) as (results agtype); +SELECT * FROM cypher('access',$$ CREATE ()-[:knows {id:2, arry:[0,1,2,3,{name: "joe"}]}]->()-[:knows {id: 3, arry:[1,3,{name:"john", stats: {age: 1000}}]}]->() $$) as (results agtype); +SELECT * FROM cypher('access', $$ MATCH (u)-[e*]->(v) RETURN e $$)as (edges agtype); +SELECT * FROM cypher('access', $$ MATCH (u)-[e*2..2]->(v) RETURN e $$)as (edges agtype); +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN properties(e[0]) $$) as (prop_first_edge agtype); +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN e[0].id $$) as (results agtype); +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN e[0].arry[2] $$) as (results agtype); +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN properties(e[1]) $$) as (prop_second_edge agtype); +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN e[1].id $$) as (results agtype); +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN e[1].arry[2] $$) as (results agtype); +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN e[1].arry[2].stats $$) as (results agtype); +SELECT * FROM cypher('access',$$ MATCH ()-[e*2..2]->() RETURN properties(e[2]) $$) as (prop_third_edge agtype); + +SELECT * FROM cypher('access',$$ MATCH ()-[e*]->() RETURN properties(e[0]), properties(e[1]) $$) as (prop_1st agtype, prop_2nd agtype); +SELECT * FROM cypher('access',$$ MATCH ()-[e*]->() RETURN e[0].id, e[1].id $$) as (results_1st agtype, results_2nd agtype); +SELECT * FROM cypher('access',$$ MATCH ()-[e*]->() RETURN e[0].arry, e[1].arry $$) as (results_1st agtype, results_2nd agtype); +SELECT * FROM cypher('access',$$ MATCH ()-[e*]->() RETURN e[0].arry[2], e[1].arry[2] $$) as (results_1st agtype, results_2nd agtype); + +SELECT drop_graph('access', true); + -- -- Clean up -- diff --git a/src/backend/utils/adt/agtype.c b/src/backend/utils/adt/agtype.c index 428d05198..6d0c5daf1 100644 --- a/src/backend/utils/adt/agtype.c +++ b/src/backend/utils/adt/agtype.c @@ -3438,67 +3438,60 @@ Datum agtype_access_operator(PG_FUNCTION_ARGS) bool *nulls = NULL; Oid *types = NULL; int nargs = 0; - agtype *object = NULL; - agtype_value *object_value = NULL; + agtype *container = NULL; + agtype_value *container_value = NULL; int i = 0; /* extract our args, we need at least 2 */ nargs = extract_variadic_args_min(fcinfo, 0, true, &args, &types, &nulls, 2); - /* return NULL if we don't have the minimum number of args */ + /* + * Return NULL if - + * + * 1) Our args are all null - nothing passed at all. + * 2) We don't have the minimum number of args. We require an object or + * an array along with either a key or element number. Note that the + * function extract_variadic_args_min will return 0 (nargs) if we + * don't have at least 2 args. + * + */ if (args == NULL || nargs == 0 || nulls[0] == true) { PG_RETURN_NULL(); } - /* get the object argument */ - object = DATUM_GET_AGTYPE_P(args[0]); - - /* if the object is a scalar, it must be a vertex or edge */ - if (AGT_ROOT_IS_SCALAR(object)) + /* check for individual NULLs */ + for (i = 0; i < nargs; i++) { - agtype_value *scalar_value = NULL; - agtype_value *property_value = NULL; - - /* unpack the scalar */ - scalar_value = get_ith_agtype_value_from_container(&object->root, 0); - - /* get the properties depending on the type or fail */ - if (scalar_value->type == AGTV_VERTEX) - { - property_value = &scalar_value->val.object.pairs[2].value; - } - else if (scalar_value->type == AGTV_EDGE) - { - property_value = &scalar_value->val.object.pairs[4].value; - } - else - { - ereport(ERROR,(errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("scalar object must be a vertex or edge"))); - } - - /* if the properties are NULL, return NULL */ - if (property_value == NULL || property_value->type == AGTV_NULL) + /* if we have a NULL, return NULL */ + if (nulls[i] == true) { PG_RETURN_NULL(); } - - /* set the object_value to the property_value. */ - object_value = property_value; } - /* check for NULL keys */ - for (i = 1; i < nargs; i++) + /* get the container argument. It could be an object or array */ + container = DATUM_GET_AGTYPE_P(args[0]); + + /* if it is a scalar, open it and pull out the value */ + if (AGT_ROOT_IS_SCALAR(container)) { - /* if we have a NULL, return NULL */ - if (nulls[i] == true) + container_value = get_ith_agtype_value_from_container(&container->root, + 0); + + /* it must be either a vertex or an edge */ + if (container_value->type != AGTV_EDGE && + container_value->type != AGTV_VERTEX) { - PG_RETURN_NULL(); + ereport(ERROR,(errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("scalar object must be a vertex or edge"))); } + + /* clear the container reference */ + container = NULL; } - /* iterate through the keys */ + /* iterate through the keys (object fields or array elements) */ for (i = 1; i < nargs; i++) { agtype *key = NULL; @@ -3513,54 +3506,68 @@ Datum agtype_access_operator(PG_FUNCTION_ARGS) errmsg("key must resolve to a scalar value"))); } + /* + * Check for a vertex or edge container_value and extract the properties + * object. + */ + if ((container_value != NULL && + (container_value->type == AGTV_EDGE || + container_value->type == AGTV_VERTEX))) + { + /* both are objects, get the properties object */ + container_value = (container_value->type == AGTV_EDGE) + ? &container_value->val.object.pairs[4].value + : &container_value->val.object.pairs[2].value; + } + /* * If we are dealing with a type of object, which can be an - * agtype OBJECT, an agtype_value OBJECT serialized (BINARY), or an * agtype_value OBJECT deserialized. */ - if ((object_value != NULL && - (object_value->type == AGTV_OBJECT || - (object_value->type == AGTV_BINARY && - AGTYPE_CONTAINER_IS_OBJECT(object_value->val.binary.data)))) || - (object != NULL && AGT_ROOT_IS_OBJECT(object))) + if ((container_value != NULL && + (container_value->type == AGTV_OBJECT || + (container_value->type == AGTV_BINARY && + AGTYPE_CONTAINER_IS_OBJECT(container_value->val.binary.data)))) || + (container != NULL && AGT_ROOT_IS_OBJECT(container))) { - object_value = execute_map_access_operator(object, object_value, - key); + container_value = execute_map_access_operator(container, + container_value, key); } /* * If we are dealing with a type of array, which can be an - * agtype ARRAY, an agtype_value ARRAY serialized (BINARY), or an * agtype_value ARRAY deserialized. */ - else if ((object_value != NULL && - (object_value->type == AGTV_ARRAY || - (object_value->type == AGTV_BINARY && - AGTYPE_CONTAINER_IS_ARRAY(object_value->val.binary.data)))) || - (object != NULL && AGT_ROOT_IS_ARRAY(object))) + else if ((container_value != NULL && + (container_value->type == AGTV_ARRAY || + (container_value->type == AGTV_BINARY && + AGTYPE_CONTAINER_IS_ARRAY(container_value->val.binary.data)))) || + (container != NULL && AGT_ROOT_IS_ARRAY(container))) { - object_value = execute_array_access_operator(object, object_value, - key); + container_value = execute_array_access_operator(container, + container_value, + key); } - /* this is unexpected */ else { + /* this is unexpected */ ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("container must be an array or object"))); } /* for NULL values return NULL */ - if (object_value == NULL || object_value->type == AGTV_NULL) + if (container_value == NULL || container_value->type == AGTV_NULL) { PG_RETURN_NULL(); } - /* clear the object reference */ - object = NULL; - + /* clear the container reference */ + container = NULL; } /* serialize and return the result */ - return AGTYPE_P_GET_DATUM(agtype_value_to_agtype(object_value)); + return AGTYPE_P_GET_DATUM(agtype_value_to_agtype(container_value)); } PG_FUNCTION_INFO_V1(agtype_access_slice);