Skip to content

Commit

Permalink
[AGE agtype_util.c] Fix issue #870 regarding orderability and added r…
Browse files Browse the repository at this point in the history 
…egression tests (#994)

Fixed issue #870
 Odd behavior in context of orderability of different agtypes.

Implemented the solution suggested here:
#870 (comment)

Clearance given here:
#870 (comment)

-Added regression tests for both comparibility and orderability
  • Loading branch information
@CapnSpek
CapnSpek authored Jun 29, 2023
1 parent d3a3bd5 commit 88ead70
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 9 deletions.
80 changes: 79 additions & 1 deletion regress/expected/agtype.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1523,7 +1523,7 @@ SELECT agtype_in('{"bool":true}') < agtype_in('{"bool":true, "null": null}');
(1 row)

-- Comparisons between types
-- Object < List < String < Boolean < Integer = Float = Numeric < Null
-- Path < Edge < Vertex < Object < List < String < Boolean < Integer = Float = Numeric < Null
SELECT agtype_in('1') < agtype_in('null');
?column?
----------
Expand Down Expand Up @@ -1590,6 +1590,24 @@ SELECT agtype_in('{"bool":true, "integer":1}') < agtype_in('{"bool":true, "integ
t
(1 row)

SELECT agtype_in('{"id":0, "label": "v", "properties":{"i":0}}::vertex') < agtype_in('{"bool":true, "i":0}');
?column?
----------
t
(1 row)

SELECT agtype_in('{"id":2, "start_id":0, "end_id":1, "label": "e", "properties":{"i":0}}::edge') < agtype_in('{"id":0, "label": "v", "properties":{"i":0}}::vertex');
?column?
----------
t
(1 row)

SELECT agtype_in('[{"id": 0, "label": "v", "properties": {"i": 0}}::vertex, {"id": 2, "start_id": 0, "end_id": 1, "label": "e", "properties": {"i": 0}}::edge, {"id": 1, "label": "v", "properties": {"i": 0}}::vertex]::path') < agtype_in('{"id":2, "start_id":0, "end_id":1, "label": "e", "properties":{"i":0}}::edge');
?column?
----------
t
(1 row)

SELECT agtype_in('1::numeric') < agtype_in('null');
?column?
----------
Expand All @@ -1602,6 +1620,66 @@ SELECT agtype_in('true') < agtype_in('1::numeric');
t
(1 row)

-- Testing orderability between types
SELECT * FROM create_graph('orderability_graph');
NOTICE: graph "orderability_graph" has been created
create_graph
--------------

(1 row)

SELECT * FROM cypher('orderability_graph', $$ CREATE (:vertex {prop: null}), (:vertex {prop: 1}), (:vertex {prop: 1.01}),(:vertex {prop: true}), (:vertex {prop:"string"}),(:vertex {prop:"string_2"}), (:vertex {prop:[1, 2, 3]}), (:vertex {prop:[1, 2, 3, 4, 5]}), (:vertex {prop:{bool:true, i:0}}), (:vertex {prop:{bool:true, i:null}}), (:vertex {prop: {id:0, label: "v", properties:{i:0}}::vertex}), (:vertex {prop: {id: 2, start_id: 0, end_id: 1, label: "e", properties: {i: 0}}::edge}), (:vertex {prop: [{id: 0, label: "v", properties: {i: 0}}::vertex, {id: 2, start_id: 0, end_id: 1, label: "e", properties: {i: 0}}::edge, {id: 1, label: "v", properties: {i: 0}}::vertex]::path}) $$) AS (x agtype);
x
---
(0 rows)

SELECT * FROM cypher('orderability_graph', $$ MATCH (n) RETURN n ORDER BY n.prop $$) AS (sorted agtype);
sorted
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
{"id": 844424930131981, "label": "vertex", "properties": {"prop": [{"id": 0, "label": "v", "properties": {"i": 0}}::vertex, {"id": 2, "label": "e", "end_id": 1, "start_id": 0, "properties": {"i": 0}}::edge, {"id": 1, "label": "v", "properties": {"i": 0}}::vertex]::path}}::vertex
{"id": 844424930131980, "label": "vertex", "properties": {"prop": {"id": 2, "label": "e", "end_id": 1, "start_id": 0, "properties": {"i": 0}}::edge}}::vertex
{"id": 844424930131979, "label": "vertex", "properties": {"prop": {"id": 0, "label": "v", "properties": {"i": 0}}::vertex}}::vertex
{"id": 844424930131978, "label": "vertex", "properties": {"prop": {"bool": true}}}::vertex
{"id": 844424930131977, "label": "vertex", "properties": {"prop": {"i": 0, "bool": true}}}::vertex
{"id": 844424930131975, "label": "vertex", "properties": {"prop": [1, 2, 3]}}::vertex
{"id": 844424930131976, "label": "vertex", "properties": {"prop": [1, 2, 3, 4, 5]}}::vertex
{"id": 844424930131973, "label": "vertex", "properties": {"prop": "string"}}::vertex
{"id": 844424930131974, "label": "vertex", "properties": {"prop": "string_2"}}::vertex
{"id": 844424930131972, "label": "vertex", "properties": {"prop": true}}::vertex
{"id": 844424930131970, "label": "vertex", "properties": {"prop": 1}}::vertex
{"id": 844424930131971, "label": "vertex", "properties": {"prop": 1.01}}::vertex
{"id": 844424930131969, "label": "vertex", "properties": {}}::vertex
(13 rows)

SELECT * FROM cypher('orderability_graph', $$ MATCH (n) RETURN n ORDER BY n.prop DESC $$) AS (sorted agtype);
sorted
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
{"id": 844424930131969, "label": "vertex", "properties": {}}::vertex
{"id": 844424930131971, "label": "vertex", "properties": {"prop": 1.01}}::vertex
{"id": 844424930131970, "label": "vertex", "properties": {"prop": 1}}::vertex
{"id": 844424930131972, "label": "vertex", "properties": {"prop": true}}::vertex
{"id": 844424930131974, "label": "vertex", "properties": {"prop": "string_2"}}::vertex
{"id": 844424930131973, "label": "vertex", "properties": {"prop": "string"}}::vertex
{"id": 844424930131976, "label": "vertex", "properties": {"prop": [1, 2, 3, 4, 5]}}::vertex
{"id": 844424930131975, "label": "vertex", "properties": {"prop": [1, 2, 3]}}::vertex
{"id": 844424930131977, "label": "vertex", "properties": {"prop": {"i": 0, "bool": true}}}::vertex
{"id": 844424930131978, "label": "vertex", "properties": {"prop": {"bool": true}}}::vertex
{"id": 844424930131979, "label": "vertex", "properties": {"prop": {"id": 0, "label": "v", "properties": {"i": 0}}::vertex}}::vertex
{"id": 844424930131980, "label": "vertex", "properties": {"prop": {"id": 2, "label": "e", "end_id": 1, "start_id": 0, "properties": {"i": 0}}::edge}}::vertex
{"id": 844424930131981, "label": "vertex", "properties": {"prop": [{"id": 0, "label": "v", "properties": {"i": 0}}::vertex, {"id": 2, "label": "e", "end_id": 1, "start_id": 0, "properties": {"i": 0}}::edge, {"id": 1, "label": "v", "properties": {"i": 0}}::vertex]::path}}::vertex
(13 rows)

SELECT * FROM drop_graph('orderability_graph', true);
NOTICE: drop cascades to 3 other objects
DETAIL: drop cascades to table orderability_graph._ag_label_vertex
drop cascades to table orderability_graph._ag_label_edge
drop cascades to table orderability_graph.vertex
NOTICE: graph "orderability_graph" has been dropped
drop_graph
------------

(1 row)

--
-- Test overloaded agytype any comparison operators =, <>, <, >, <=, >=,
--
Expand Down
11 changes: 10 additions & 1 deletion regress/sql/agtype.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ SELECT agtype_in('{"bool":true, "null": null}') = agtype_in('{"null":null, "bool
SELECT agtype_in('{"bool":true}') < agtype_in('{"bool":true, "null": null}');

-- Comparisons between types
-- Object < List < String < Boolean < Integer = Float = Numeric < Null
-- Path < Edge < Vertex < Object < List < String < Boolean < Integer = Float = Numeric < Null
SELECT agtype_in('1') < agtype_in('null');
SELECT agtype_in('NaN') < agtype_in('null');
SELECT agtype_in('Infinity') < agtype_in('null');
Expand All @@ -415,8 +415,17 @@ SELECT agtype_in('[1,3,5,7,9,11]') < agtype_in('"string"');
SELECT agtype_in('{"bool":true, "integer":1}') < agtype_in('[1,3,5,7,9,11]');
SELECT agtype_in('[1, "string"]') < agtype_in('[1, 1]');
SELECT agtype_in('{"bool":true, "integer":1}') < agtype_in('{"bool":true, "integer":null}');
SELECT agtype_in('{"id":0, "label": "v", "properties":{"i":0}}::vertex') < agtype_in('{"bool":true, "i":0}');
SELECT agtype_in('{"id":2, "start_id":0, "end_id":1, "label": "e", "properties":{"i":0}}::edge') < agtype_in('{"id":0, "label": "v", "properties":{"i":0}}::vertex');
SELECT agtype_in('[{"id": 0, "label": "v", "properties": {"i": 0}}::vertex, {"id": 2, "start_id": 0, "end_id": 1, "label": "e", "properties": {"i": 0}}::edge, {"id": 1, "label": "v", "properties": {"i": 0}}::vertex]::path') < agtype_in('{"id":2, "start_id":0, "end_id":1, "label": "e", "properties":{"i":0}}::edge');
SELECT agtype_in('1::numeric') < agtype_in('null');
SELECT agtype_in('true') < agtype_in('1::numeric');
-- Testing orderability between types
SELECT * FROM create_graph('orderability_graph');
SELECT * FROM cypher('orderability_graph', $$ CREATE (:vertex {prop: null}), (:vertex {prop: 1}), (:vertex {prop: 1.01}),(:vertex {prop: true}), (:vertex {prop:"string"}),(:vertex {prop:"string_2"}), (:vertex {prop:[1, 2, 3]}), (:vertex {prop:[1, 2, 3, 4, 5]}), (:vertex {prop:{bool:true, i:0}}), (:vertex {prop:{bool:true, i:null}}), (:vertex {prop: {id:0, label: "v", properties:{i:0}}::vertex}), (:vertex {prop: {id: 2, start_id: 0, end_id: 1, label: "e", properties: {i: 0}}::edge}), (:vertex {prop: [{id: 0, label: "v", properties: {i: 0}}::vertex, {id: 2, start_id: 0, end_id: 1, label: "e", properties: {i: 0}}::edge, {id: 1, label: "v", properties: {i: 0}}::vertex]::path}) $$) AS (x agtype);
SELECT * FROM cypher('orderability_graph', $$ MATCH (n) RETURN n ORDER BY n.prop $$) AS (sorted agtype);
SELECT * FROM cypher('orderability_graph', $$ MATCH (n) RETURN n ORDER BY n.prop DESC $$) AS (sorted agtype);
SELECT * FROM drop_graph('orderability_graph', true);

--
-- Test overloaded agytype any comparison operators =, <>, <, >, <=, >=,
Expand Down
48 changes: 41 additions & 7 deletions src/backend/utils/adt/agtype_util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,20 +197,42 @@ uint32 get_agtype_length(const agtype_container *agtc, int index)
*/
static int get_type_sort_priority(enum agtype_value_type type)
{
if (type == AGTV_OBJECT)
if (type == AGTV_PATH)
{
return 0;
if (type == AGTV_VERTEX)
}
if (type == AGTV_EDGE)
{
return 1;
if (type == AGTV_ARRAY)
}
if (type == AGTV_VERTEX)
{
return 2;
if (type == AGTV_STRING)
}
if (type == AGTV_OBJECT)
{
return 3;
if (type == AGTV_BOOL)
}
if (type == AGTV_ARRAY)
{
return 4;
if (type == AGTV_NUMERIC || type == AGTV_INTEGER || type == AGTV_FLOAT)
}
if (type == AGTV_STRING)
{
return 5;
if (type == AGTV_NULL)
}
if (type == AGTV_BOOL)
{
return 6;
}
if (type == AGTV_NUMERIC || type == AGTV_INTEGER || type == AGTV_FLOAT)
{
return 7;
}
if (type == AGTV_NULL)
{
return 8;
}
return -1;
}

Expand Down Expand Up @@ -356,6 +378,18 @@ int compare_agtype_containers_orderability(agtype_container *a,
break;
}

/* Correction step because AGTV_ARRAY might be there just because of the container type */
/* Case 1: left side is assigned to an array, right is an object */
if(va.type == AGTV_ARRAY && vb.type == AGTV_OBJECT)
{
ra = agtype_iterator_next(&ita, &va, false);
}
/* Case 2: left side is an object, right side is assigned to an array */
else if(va.type == AGTV_OBJECT && vb.type == AGTV_ARRAY)
{
rb = agtype_iterator_next(&itb, &vb, false);
}

Assert(va.type != vb.type);
Assert(va.type != AGTV_BINARY);
Assert(vb.type != AGTV_BINARY);
Expand Down

0 comments on commit 88ead70

Please sign in to comment.