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

[M4rcxs]

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

[M4rcxs]

The solution that I came up with is to use the pstrdup function, which allocates a new memory block for the string and then copies the content of the buffer into it.

The Pull Request: #1084

[jrgemignani]

@M4rcxs Please go back and correct all of toStringList, and related PRs. Please make sure you are compiling these locally with the correct flags.

src/backend/utils/adt/agtype.c: In function ‘age_tostringlist’:
src/backend/utils/adt/agtype.c:6278:13: warning: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘int64’ [-Wformat=]
             sprintf(buffer, "%d", elem->val.int_value);
             ^
src/backend/utils/adt/agtype.c:6213:11: warning: variable ‘float_num’ set but not used [-Wunused-but-set-variable]
     float float_num;
           ^

[M4rcxs]

@M4rcxs Please go back and correct all of toStringList, and related PRs. Please make sure you are compiling these locally with the correct flags.

src/backend/utils/adt/agtype.c: In function ‘age_tostringlist’:
src/backend/utils/adt/agtype.c:6278:13: warning: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘int64’ [-Wformat=]
             sprintf(buffer, "%d", elem->val.int_value);
             ^
src/backend/utils/adt/agtype.c:6213:11: warning: variable ‘float_num’ set but not used [-Wunused-but-set-variable]
     float float_num;
           ^

The solution I came up is change from %d to %ld, also remove the lines using float_num variable.

I'll open refacting PR's soon, thanks for your review!

[jrgemignani]

Ty!

Btw, with regards to using float,... agtype's float is a float8. So, when you do refer to agtype's float, make sure to use float8 :)

[M4rcxs]

Ty!

Btw, with regards to using float,... agtype's float is a float8. So, when you do refer to agtype's float, make sure to use float8 :)

Thank you! done it at #1128