CapnSpek commented on PR #994:
URL: https://github.com/apache/age/pull/994#issuecomment-1607713577
I ran into a problem while testing it: -
Even after changing the priorities in the `get_type_sort_priority`,
everything is good except for every query, object is the smallest agtype there
is.
While debugging, I discovered this issue does not arise from that function,
but instead from `compare_agtype_containers_orderability` function
(agtype_util.c: line 231)
```
int compare_agtype_containers_orderability(agtype_container *a,
agtype_container *b)
{
agtype_iterator *ita;
agtype_iterator *itb;
int res = 0;
ita = agtype_iterator_init(a);
itb = agtype_iterator_init(b);
```
The function takes 2 agtype_container values as parameters (the two values
to be compared).
Then it initializes an iterator for both, and declares the result variable
in which the result of the comparison will be stored, then returned.
The issue takes place in the do-while loop: -
```
do
{
agtype_value va;
agtype_value vb;
agtype_iterator_token ra;
agtype_iterator_token rb;
ra = agtype_iterator_next(&ita, &va, false);
rb = agtype_iterator_next(&itb, &vb, false);
```
The bottom two lines of the code above set both `va.type` & `vb.type` to
`AGTV_ARRAY` in case of all agtypes except object, in which they set it to
`AGTV_OBJECT`
The wrong `AGTV_ARRAY` assignment to all values only takes place in the
first iteration of the loop. In the second iteration it corrects itself to the
correct type.
The second iteration only takes place if both values have same agtype, thus
this correction step is skipped when `AGTV_OBJECT` and `AGTV_ARRAY` are
compared, and decision is readily made that `AGTV_ARRAY > AGTV_OBJECT`.
I believe it might be because `agtype_container` is an `ARRAY` or `OBJECT`
type struct as written in its comment in agtype.h: line 224: -
```
/*
* An agtype array or object node, within an agtype Datum.
*
* An array has one child for each element, stored in array order.
*
* An object has two children for each key/value pair. The keys all appear
* first, in key sort order; then the values appear, in an order matching the
* key order. This arrangement keeps the keys compact in memory, making a
* search for a particular key more cache-friendly.
*/
typedef struct agtype_container
{
uint32 header; /* number of elements or key/value pairs, and flags */
agtentry children[FLEXIBLE_ARRAY_MEMBER];
/* the data for each child node follows. */
} agtype_container;
```
Solution: -
Because it is a very special case (when both Agtypes differ, one is an
array, other is an object)
The correction can be made into a special case below line 370 as follows
```
if(va.type == AGTV_ARRAY && vb.type == AGTV_OBJECT)
{
ra = agtype_iterator_next(&ita, &va, false);
}
else if(va.type == AGTV_OBJECT && vb.type == AGTV_ARRAY)
{
rb = agtype_iterator_next(&itb, &vb, false);
}
```
That fixes the problem without any side effects.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
