Inserting values of different types in an array, transforms them into wrong data

Question

I've created a function in C language to AGE that converts a list of elements into a new list of string elements, based in toStringList of opencypher.

When I insert diferent data types in an array in a C inside my AGE, the returned array modifies the elements from the original array to be equal to the last element:

demo=# SELECT * FROM cypher('grafo', $$
    RETURN toStringList([1.3, 8, 7.4, 2.5])
$$) AS (toFloatList agtype);
        tofloatlist
----------------------------
 ["2.5", "2", "2.5", "2.5"]
(1 row)

The toStringList() function:

PG_FUNCTION_INFO_V1(age_tostringlist);
/*
 * toStringList() converts a list of values and returns a list of String values. 
 * If any values are not convertible to string point they will be null in the list returned.
 */
Datum age_tostringlist(PG_FUNCTION_ARGS)
{
    agtype *agt_arg = NULL;
    agtype_in_state agis_result;
    agtype_value *elem;
    agtype_value string_elem;
    char *string = NULL;
    int count;
    int i;
    float float_num;
    char buffer[64];

    /* check for null */
    if (PG_ARGISNULL(0))
    {
        PG_RETURN_NULL();
    }
    agt_arg = AG_GET_ARG_AGTYPE_P(0);
    /* check for an array */
    if (!AGT_ROOT_IS_ARRAY(agt_arg) || AGT_ROOT_IS_SCALAR(agt_arg))
        ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                        errmsg("toStringList() argument must resolve to a list or null")));

    count = AGT_ROOT_COUNT(agt_arg);

    /* if we have an empty list or only one element in the list, return null */
    if (count == 0)
        PG_RETURN_NULL();

    /* clear the result structure */
    MemSet(&agis_result, 0, sizeof(agtype_in_state));

    /* push the beginning of the array */
    agis_result.res = push_agtype_value(&agis_result.parse_state,
                                        WAGT_BEGIN_ARRAY, NULL);

    /* iterate through the list */
    for (i = 0; i < count; i++)
    {
        // TODO: check element's type, it's value, and convert it to string if possible.
        elem = get_ith_agtype_value_from_container(&agt_arg->root, i);
        string_elem.type = AGTV_STRING;

        switch (elem->type)
        {
        case AGTV_STRING:

            if(!elem)
            {
                string_elem.type = AGTV_NULL;

                agis_result.res = push_agtype_value(&agis_result.parse_state, WAGT_ELEM, &string_elem);
            }
            string_elem.val.string.val = elem->val.string.val;
            string_elem.val.string.len = elem->val.string.len;

            agis_result.res = push_agtype_value(&agis_result.parse_state, WAGT_ELEM, &string_elem);
            
            break;

        case AGTV_FLOAT:

            string_elem.type = AGTV_STRING;
            // sprintf(buffer, "%d", elem->val.float_value);
            float_num = elem->val.float_value;
            string_elem.val.string.val = gcvt(float_num, 6, buffer);
            string_elem.val.string.len = strlen(buffer);

            agis_result.res = push_agtype_value(&agis_result.parse_state, WAGT_ELEM, &string_elem);

            break; 

        case AGTV_INTEGER:

            string_elem.type = AGTV_STRING;
            sprintf(buffer, "%d", elem->val.int_value);
            string_elem.val.string.val = buffer;
            string_elem.val.string.len = strlen(buffer);

            agis_result.res = push_agtype_value(&agis_result.parse_state, WAGT_ELEM, &string_elem);
        
            break;
        
        default:

            string_elem.type = AGTV_NULL;
            agis_result.res = push_agtype_value(&agis_result.parse_state, WAGT_ELEM, &string_elem);

            break;
        }
    }
    agis_result.res = push_agtype_value(&agis_result.parse_state, WAGT_END_ARRAY, NULL);

    PG_RETURN_POINTER(agtype_value_to_agtype(agis_result.res));
}

The age--1.3.0.sql file

CREATE FUNCTION ag_catalog.age_tostringlist(variadic "any")
RETURNS agtype
LANGUAGE c
IMMUTABLE
RETURNS NULL ON NULL INPUT
PARALLEL SAFE
AS 'MODULE_PATHNAME';

If someone knows how can I solve this I would be grateful for any kind of help or advice

Can you add to the question the part of the declaration which is added on the age---.sql script of the function so that it be easy to reproduce the problem with the same as your side? — Mohamed Mokhtar - rrrokhtar, Jul 21 '23 at 09:09

score 0 · Answer 1 · answered Jul 21 '23 at 22:09

It is not a solution but is an observation:

There is an issue with your code that buffer is being used is being overwritten every time you use it to parse the float/integer to the string it works on the same memory address buffer and that line makes it refers to the same address every time it goes into so that the latest parsed value it set to all of the old entries and those got changed

 string_elem.val.string.val = gcvt(float_num, 6, buffer);

In your given example all set to the last parsed number with restriction of the length of the string, because the second element was a single digit it has taken only the first char of the last parsed input

score 0 · Answer 2 · answered Jul 22 '23 at 08:08

I think the problem lies in this line: string_elem.val.string.val = buffer;, and each iteration of the loop overwrite the contents of the buffer. When you assign string_elem.val.string.val equal to the buffer, it is not making a copy of the data instead pointing to the buffer's memory location.

score 0 · Answer 3 · answered Jul 29 '23 at 13:34

You are using a single buffer for all elements and this ends with the last value of the element converted. You need to allocate different memories for each element for it to be successful.

You should use the pstrdup function in Postgres for this. It makes sure all elements have separate memory allocation and is freed when no longer useful.

For AGTV_STRING:

string_elem.val.string.val = pstrdup(elem->val.string.val, elem->val.string.len);

For AGTV_FLOAT:

sprintf(buffer, sizeof(buffer), "%f", float_num);
string_elem.val.string.val = pstrdup(buffer);

For AGTV_INTEGER:

sprintf(buffer, sizeof(buffer), "%d", elem->val.int_value);
string_elem.val.string.val = pstrdup(buffer);

Also, for the FLOAT case, I used sprintf instead of gvct because it prevents buffer overflow and controls buffer size better.

Inserting values of different types in an array, transforms them into wrong data

3 Answers3