From 85dcd889c134628f175b493d6d431c9660f332a0 Mon Sep 17 00:00:00 2001 From: John Gemignani Date: Mon, 9 Sep 2024 09:17:42 -0700 Subject: [PATCH] Fix memory leaks in functions part 1 (#2066) (#2087) Fixing memory leaks in functions due to contexts not clearing out the memory soon enough. Background: Destroying a context will free up its associated memory. However, before that happens, a process can still accumulate a lot of memory working on large data sets if it expects the context free to do it for it. For example, when PG does quicksort the context isn't destroyed until after the qs has finished. This can cause memory to be exhausted. Put in more aggressive freeing of memory. However, there are a lot of areas that need this applied. So, this is part 1 of at least 3 or 4 parts. Worked mainly on `agtype.c` However, dealt with linked functions in other files. modified: src/backend/utils/adt/age_vle.c modified: src/backend/utils/adt/agtype.c modified: src/backend/utils/adt/agtype_util.c modified: src/include/utils/agtype.h Resolved Conflicts: src/backend/utils/adt/agtype.c --- src/backend/utils/adt/age_vle.c | 1 - src/backend/utils/adt/agtype.c | 417 +++++++++++++++++++++------- src/backend/utils/adt/agtype_util.c | 3 +- src/include/utils/agtype.h | 1 + 4 files changed, 325 insertions(+), 97 deletions(-) diff --git a/src/backend/utils/adt/age_vle.c b/src/backend/utils/adt/age_vle.c index 40d04fb3e..8e7e1ac49 100644 --- a/src/backend/utils/adt/age_vle.c +++ b/src/backend/utils/adt/age_vle.c @@ -2148,7 +2148,6 @@ agtype_value *agtv_materialize_vle_edges(agtype *agt_arg_vpc) /* build the AGTV_ARRAY of edges from the VLE_path_container */ agtv_array = build_edge_list(vpc); - /* convert the agtype_value to agtype and return it */ return agtv_array; } diff --git a/src/backend/utils/adt/agtype.c b/src/backend/utils/adt/agtype.c index 974184724..6deeecf2b 100644 --- a/src/backend/utils/adt/agtype.c +++ b/src/backend/utils/adt/agtype.c @@ -238,8 +238,11 @@ PG_FUNCTION_INFO_V1(graphid_recv); Datum graphid_recv(PG_FUNCTION_ARGS) { StringInfo buf = (StringInfo) PG_GETARG_POINTER(0); + int64 result = pq_getmsgint64(buf); - PG_RETURN_INT64(pq_getmsgint64(buf)); + PG_FREE_IF_COPY(buf, 0); + + PG_RETURN_INT64(result); } /* @@ -277,6 +280,7 @@ Datum agtype_recv(PG_FUNCTION_ARGS) int version = pq_getmsgint(buf, 1); char *str = NULL; int nbytes = 0; + Datum result; if (version == 1) { @@ -287,7 +291,12 @@ Datum agtype_recv(PG_FUNCTION_ARGS) elog(ERROR, "unsupported agtype version number %d", version); } - return agtype_from_cstring(str, nbytes); + result = agtype_from_cstring(str, nbytes); + + PG_FREE_IF_COPY(buf, 0); + pfree(str); + + return result; } /* @@ -313,6 +322,8 @@ Datum agtype_send(PG_FUNCTION_ARGS) pfree(agtype_text->data); pfree(agtype_text); + PG_FREE_IF_COPY(agt, 0); + PG_RETURN_BYTEA_P(pq_endtypsend(&buf)); } @@ -324,8 +335,11 @@ PG_FUNCTION_INFO_V1(agtype_in); Datum agtype_in(PG_FUNCTION_ARGS) { char *str = PG_GETARG_CSTRING(0); + Datum result = agtype_from_cstring(str, strlen(str)); - return agtype_from_cstring(str, strlen(str)); + PG_FREE_IF_COPY(str, 0); + + return result; } PG_FUNCTION_INFO_V1(agtype_out); @@ -342,6 +356,8 @@ Datum agtype_out(PG_FUNCTION_ARGS) out = agtype_to_cstring(NULL, &agt->root, VARSIZE(agt)); + PG_FREE_IF_COPY(agt, 0); + PG_RETURN_CSTRING(out); } @@ -2129,6 +2145,7 @@ Datum _agtype_build_path(PG_FUNCTION_ARGS) Datum make_path(List *path) { + agtype *agt_result; ListCell *lc; agtype_in_state result; int i = 1; @@ -2154,7 +2171,7 @@ Datum make_path(List *path) foreach (lc, path) { - agtype *agt= DATUM_GET_AGTYPE_P(PointerGetDatum(lfirst(lc))); + agtype *agt = DATUM_GET_AGTYPE_P(PointerGetDatum(lfirst(lc))); agtype_value *elem; elem = get_ith_agtype_value_from_container(&agt->root, 0); @@ -2179,6 +2196,12 @@ Datum make_path(List *path) add_agtype((Datum)agt, false, &result, AGTYPEOID, false); + if ((Pointer) (agt) != lfirst(lc)) + { + pfree(agt); + } + pfree_agtype_value(elem); + i++; } @@ -2186,7 +2209,11 @@ Datum make_path(List *path) result.res->type = AGTV_PATH; - PG_RETURN_POINTER(agtype_value_to_agtype(result.res)); + agt_result = agtype_value_to_agtype(result.res); + + pfree_agtype_in_state(&result); + + PG_RETURN_POINTER(agt_result); } PG_FUNCTION_INFO_V1(_agtype_build_vertex); @@ -2253,6 +2280,9 @@ Datum _agtype_build_vertex(PG_FUNCTION_ARGS) rawscalar = build_agtype(bstate); pfree_agtype_build_state(bstate); + PG_FREE_IF_COPY(label, 1); + PG_FREE_IF_COPY(properties, 2); + PG_RETURN_POINTER(rawscalar); } @@ -2352,6 +2382,10 @@ Datum _agtype_build_edge(PG_FUNCTION_ARGS) write_extended(bstate, edge, AGT_HEADER_EDGE); rawscalar = build_agtype(bstate); pfree_agtype_build_state(bstate); + + PG_FREE_IF_COPY(label, 3); + PG_FREE_IF_COPY(properties, 4); + PG_RETURN_POINTER(rawscalar); } @@ -2470,13 +2504,18 @@ PG_FUNCTION_INFO_V1(agtype_build_map_noargs); Datum agtype_build_map_noargs(PG_FUNCTION_ARGS) { agtype_in_state result; + agtype *agt_result; memset(&result, 0, sizeof(agtype_in_state)); push_agtype_value(&result.parse_state, WAGT_BEGIN_OBJECT, NULL); result.res = push_agtype_value(&result.parse_state, WAGT_END_OBJECT, NULL); - PG_RETURN_POINTER(agtype_value_to_agtype(result.res)); + agt_result = agtype_value_to_agtype(result.res); + + pfree_agtype_in_state(&result); + + PG_RETURN_POINTER(agt_result); } PG_FUNCTION_INFO_V1(agtype_build_map_nonull); @@ -2487,6 +2526,7 @@ PG_FUNCTION_INFO_V1(agtype_build_map_nonull); Datum agtype_build_map_nonull(PG_FUNCTION_ARGS) { agtype_value *result = NULL; + agtype *agt_result; result = agtype_build_map_as_agtype_value(fcinfo); if (result == NULL) @@ -2495,8 +2535,11 @@ Datum agtype_build_map_nonull(PG_FUNCTION_ARGS) } remove_null_from_agtype_object(result); + agt_result = agtype_value_to_agtype(result); + + pfree_agtype_value(result); - PG_RETURN_POINTER(agtype_value_to_agtype(result)); + PG_RETURN_POINTER(agt_result); } PG_FUNCTION_INFO_V1(agtype_build_list); @@ -2512,12 +2555,15 @@ Datum agtype_build_list(PG_FUNCTION_ARGS) Datum *args; bool *nulls; Oid *types; + agtype *agt_result; /*build argument values to build the array */ nargs = extract_variadic_args(fcinfo, 0, true, &args, &types, &nulls); if (nargs < 0) + { PG_RETURN_NULL(); + } memset(&result, 0, sizeof(agtype_in_state)); @@ -2525,11 +2571,17 @@ Datum agtype_build_list(PG_FUNCTION_ARGS) NULL); for (i = 0; i < nargs; i++) + { add_agtype(args[i], nulls[i], &result, types[i], false); + PG_FREE_IF_COPY(DatumGetPointer(args[i]), i); + } result.res = push_agtype_value(&result.parse_state, WAGT_END_ARRAY, NULL); + agt_result = agtype_value_to_agtype(result.res); + + pfree_agtype_in_state(&result); - PG_RETURN_POINTER(agtype_value_to_agtype(result.res)); + PG_RETURN_POINTER(agt_result); } PG_FUNCTION_INFO_V1(agtype_build_list_noargs); @@ -2540,13 +2592,18 @@ PG_FUNCTION_INFO_V1(agtype_build_list_noargs); Datum agtype_build_list_noargs(PG_FUNCTION_ARGS) { agtype_in_state result; + agtype *agt_result; memset(&result, 0, sizeof(agtype_in_state)); push_agtype_value(&result.parse_state, WAGT_BEGIN_ARRAY, NULL); result.res = push_agtype_value(&result.parse_state, WAGT_END_ARRAY, NULL); - PG_RETURN_POINTER(agtype_value_to_agtype(result.res)); + agt_result = agtype_value_to_agtype(result.res); + + pfree_agtype_in_state(&result); + + PG_RETURN_POINTER(agt_result); } /* @@ -2659,6 +2716,7 @@ Datum agtype_to_int8(PG_FUNCTION_ARGS) agtype_value *container = NULL; int64 result = 0x0; agtype *arg_agt = NULL; + bool is_scalar = false; /* get the agtype equivalence of any convertable input type */ arg_agt = get_one_agtype_from_variadic_args(fcinfo, 0, 1); @@ -2669,7 +2727,11 @@ Datum agtype_to_int8(PG_FUNCTION_ARGS) PG_RETURN_NULL(); } - if (!agtype_extract_scalar(&arg_agt->root, &agtv) || + /* get the scalar value if it is one and set the flag accordingly */ + is_scalar = agtype_extract_scalar(&arg_agt->root, &agtv); + + /* if it isn't something that can be cast error out */ + if (!is_scalar || (agtv.type != AGTV_FLOAT && agtv.type != AGTV_INTEGER && agtv.type != AGTV_NUMERIC && @@ -2706,7 +2768,7 @@ Datum agtype_to_int8(PG_FUNCTION_ARGS) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid agtype string to int8 type: %d", - (int)temp->type))); + temp->type))); } /* save the top agtype_value */ @@ -2725,7 +2787,7 @@ Datum agtype_to_int8(PG_FUNCTION_ARGS) else { elog(ERROR, "unexpected string type: %d in agtype_to_int8", - (int)temp->type); + temp->type); } } @@ -2753,7 +2815,7 @@ Datum agtype_to_int8(PG_FUNCTION_ARGS) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid conversion type in agtype_to_int8: %d", - (int)agtv_p->type))); + agtv_p->type))); } /* free the container, if it was used */ @@ -3089,18 +3151,25 @@ Datum agtype_to_text(PG_FUNCTION_ARGS) /* Return null if arg_agt is null. This covers SQL and Agtype NULLS */ if (arg_agt == NULL) + { PG_RETURN_NULL(); + } /* check that we have a scalar value */ if (!AGT_ROOT_IS_SCALAR(arg_agt)) + { ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("agtype argument must resolve to a scalar value"))); + } /* get the arg parameter */ arg_value = get_ith_agtype_value_from_container(&arg_agt->root, 0); + PG_FREE_IF_COPY(arg_agt, 0); text_value = agtype_value_to_text(arg_value, true); + pfree_agtype_value(arg_value); + if (text_value == NULL) { PG_RETURN_NULL(); @@ -3305,6 +3374,7 @@ static agtype_value *execute_array_access_operator(agtype *array, agtype *array_index) { agtype_value *array_index_value = NULL; + agtype_value *result = NULL; /* unpack the array index value */ array_index_value = get_ith_agtype_value_from_container(&array_index->root, @@ -3313,6 +3383,7 @@ static agtype_value *execute_array_access_operator(agtype *array, /* if AGTV_NULL return NULL */ if (array_index_value->type == AGTV_NULL) { + pfree_agtype_value(array_index_value); return NULL; } @@ -3323,8 +3394,11 @@ static agtype_value *execute_array_access_operator(agtype *array, (errmsg("array index must resolve to an integer value"))); } - return execute_array_access_operator_internal( - array, array_value, array_index_value->val.int_value); + result = execute_array_access_operator_internal(array, array_value, + array_index_value->val.int_value); + + pfree_agtype_value(array_index_value); + return result; } static agtype_value *execute_array_access_operator_internal(agtype *array, @@ -3699,32 +3773,44 @@ Datum agtype_object_field_agtype(PG_FUNCTION_ARGS) { agtype *agt = AG_GET_ARG_AGTYPE_P(0); agtype *key = AG_GET_ARG_AGTYPE_P(1); - agtype_value *key_value; - if (!AGT_ROOT_IS_SCALAR(key)) + if (AGT_ROOT_IS_SCALAR(key)) { - PG_RETURN_NULL(); - } + agtype_value *key_value; - key_value = get_ith_agtype_value_from_container(&key->root, 0); + key_value = get_ith_agtype_value_from_container(&key->root, 0); - if (key_value->type == AGTV_INTEGER) - { - PG_RETURN_TEXT_P(agtype_array_element_impl(fcinfo, agt, + if (key_value->type == AGTV_INTEGER || + key_value->type == AGTV_STRING) + { + Datum retval = 0; + + if (key_value->type == AGTV_INTEGER) + { + retval = agtype_array_element_impl(fcinfo, agt, key_value->val.int_value, - false)); - } - else if (key_value->type == AGTV_STRING) - { - AG_RETURN_AGTYPE_P(agtype_object_field_impl(fcinfo, agt, - key_value->val.string.val, - key_value->val.string.len, - false)); - } - else - { - PG_RETURN_NULL(); + false); + } + else if (key_value->type == AGTV_STRING) + { + retval = agtype_object_field_impl(fcinfo, agt, + key_value->val.string.val, + key_value->val.string.len, + false); + } + + pfree_agtype_value(key_value); + PG_FREE_IF_COPY(agt, 0); + PG_FREE_IF_COPY(key, 1); + + PG_RETURN_POINTER((const void*) retval); + } + pfree_agtype_value(key_value); } + + PG_FREE_IF_COPY(agt, 0); + PG_FREE_IF_COPY(key, 1); + PG_RETURN_NULL(); } PG_FUNCTION_INFO_V1(agtype_object_field_text_agtype); @@ -3733,32 +3819,43 @@ Datum agtype_object_field_text_agtype(PG_FUNCTION_ARGS) { agtype *agt = AG_GET_ARG_AGTYPE_P(0); agtype *key = AG_GET_ARG_AGTYPE_P(1); - agtype_value *key_value; - if (!AGT_ROOT_IS_SCALAR(key)) + if (AGT_ROOT_IS_SCALAR(key)) { - PG_RETURN_NULL(); - } + agtype_value *key_value; - key_value = get_ith_agtype_value_from_container(&key->root, 0); + key_value = get_ith_agtype_value_from_container(&key->root, 0); - if (key_value->type == AGTV_INTEGER) - { - PG_RETURN_TEXT_P(agtype_array_element_impl(fcinfo, agt, + if (key_value->type == AGTV_INTEGER || key_value->type == AGTV_STRING) + { + Datum retval = 0; + + if (key_value->type == AGTV_INTEGER) + { + retval = agtype_array_element_impl(fcinfo, agt, key_value->val.int_value, - true)); - } - else if (key_value->type == AGTV_STRING) - { - AG_RETURN_AGTYPE_P(agtype_object_field_impl(fcinfo, agt, - key_value->val.string.val, - key_value->val.string.len, - true)); - } - else - { - PG_RETURN_NULL(); + true); + } + else if (key_value->type == AGTV_STRING) + { + retval = agtype_object_field_impl(fcinfo, agt, + key_value->val.string.val, + key_value->val.string.len, + true); + } + + pfree_agtype_value(key_value); + PG_FREE_IF_COPY(agt, 0); + PG_FREE_IF_COPY(key, 1); + + PG_RETURN_POINTER((const void*) retval); + } + pfree_agtype_value(key_value); } + + PG_FREE_IF_COPY(agt, 0); + PG_FREE_IF_COPY(key, 1); + PG_RETURN_NULL(); } PG_FUNCTION_INFO_V1(agtype_object_field); @@ -3767,10 +3864,14 @@ Datum agtype_object_field(PG_FUNCTION_ARGS) { agtype *agt = AG_GET_ARG_AGTYPE_P(0); text *key = PG_GETARG_TEXT_PP(1); + Datum retval; - AG_RETURN_AGTYPE_P(agtype_object_field_impl(fcinfo, agt, VARDATA_ANY(key), - VARSIZE_ANY_EXHDR(key), - false)); + retval = agtype_object_field_impl(fcinfo, agt, VARDATA_ANY(key), + VARSIZE_ANY_EXHDR(key), false); + PG_FREE_IF_COPY(agt, 0); + PG_FREE_IF_COPY(key, 1); + + AG_RETURN_AGTYPE_P((const void*) retval); } PG_FUNCTION_INFO_V1(agtype_object_field_text); @@ -3779,9 +3880,14 @@ Datum agtype_object_field_text(PG_FUNCTION_ARGS) { agtype *agt = AG_GET_ARG_AGTYPE_P(0); text *key = PG_GETARG_TEXT_PP(1); + Datum retval; + + retval = agtype_object_field_impl(fcinfo, agt, VARDATA_ANY(key), + VARSIZE_ANY_EXHDR(key), true); + PG_FREE_IF_COPY(agt, 0); + PG_FREE_IF_COPY(key, 1); - PG_RETURN_TEXT_P(agtype_object_field_impl(fcinfo, agt, VARDATA_ANY(key), - VARSIZE_ANY_EXHDR(key), true)); + PG_RETURN_TEXT_P((const void*) retval); } PG_FUNCTION_INFO_V1(agtype_array_element); @@ -3790,8 +3896,13 @@ Datum agtype_array_element(PG_FUNCTION_ARGS) { agtype *agt = AG_GET_ARG_AGTYPE_P(0); int elem = PG_GETARG_INT32(1); + Datum retval; + + retval = agtype_array_element_impl(fcinfo, agt, elem, false); - AG_RETURN_AGTYPE_P(agtype_array_element_impl(fcinfo, agt, elem, false)); + PG_FREE_IF_COPY(agt, 0); + + AG_RETURN_AGTYPE_P((const void*) retval); } PG_FUNCTION_INFO_V1(agtype_array_element_text); @@ -3800,8 +3911,13 @@ Datum agtype_array_element_text(PG_FUNCTION_ARGS) { agtype *agt = AG_GET_ARG_AGTYPE_P(0); int elem = PG_GETARG_INT32(1); + Datum retval; + + retval = agtype_array_element_impl(fcinfo, agt, elem, true); + + PG_FREE_IF_COPY(agt, 0); - PG_RETURN_TEXT_P(agtype_array_element_impl(fcinfo, agt, elem, true)); + PG_RETURN_TEXT_P((const void*) retval); } PG_FUNCTION_INFO_V1(agtype_access_operator); @@ -3817,6 +3933,7 @@ Datum agtype_access_operator(PG_FUNCTION_ARGS) int nargs = 0; agtype *container = NULL; agtype_value *container_value = NULL; + agtype *result = NULL; int i = 0; /* extract our args, we need at least 2 */ @@ -3834,6 +3951,10 @@ Datum agtype_access_operator(PG_FUNCTION_ARGS) */ if (args == NULL || nargs == 0 || nulls[0] == true) { + pfree(args); + pfree(types); + pfree(nulls); + PG_RETURN_NULL(); } @@ -3843,6 +3964,9 @@ Datum agtype_access_operator(PG_FUNCTION_ARGS) /* if we have a NULL, return NULL */ if (nulls[i] == true) { + pfree(args); + pfree(types); + pfree(nulls); PG_RETURN_NULL(); } } @@ -3858,6 +3982,7 @@ Datum agtype_access_operator(PG_FUNCTION_ARGS) /* retrieve an array of edges from the vpc */ container_value = agtv_materialize_vle_edges(container); /* clear the container reference */ + container = NULL; } else @@ -3960,8 +4085,14 @@ Datum agtype_access_operator(PG_FUNCTION_ARGS) container = NULL; } + pfree(args); + pfree(types); + pfree(nulls); + /* serialize and return the result */ - return AGTYPE_P_GET_DATUM(agtype_value_to_agtype(container_value)); + result = agtype_value_to_agtype(container_value); + + return AGTYPE_P_GET_DATUM(result); } PG_FUNCTION_INFO_V1(agtype_access_slice); @@ -3973,7 +4104,10 @@ Datum agtype_access_slice(PG_FUNCTION_ARGS) agtype_value *lidx_value = NULL; agtype_value *uidx_value = NULL; agtype_in_state result; + agtype *agt_result = NULL; agtype *agt_array = NULL; + agtype *agt_lidx = NULL; + agtype *agt_uidx = NULL; agtype_value *agtv_array = NULL; int64 upper_index = 0; int64 lower_index = 0; @@ -4022,8 +4156,8 @@ Datum agtype_access_slice(PG_FUNCTION_ARGS) } else { - lidx_value = get_ith_agtype_value_from_container( - &(AG_GET_ARG_AGTYPE_P(1))->root, 0); + agt_lidx = AG_GET_ARG_AGTYPE_P(1); + lidx_value = get_ith_agtype_value_from_container(&agt_lidx->root, 0); /* adjust for AGTV_NULL */ if (lidx_value->type == AGTV_NULL) { @@ -4039,8 +4173,8 @@ Datum agtype_access_slice(PG_FUNCTION_ARGS) } else { - uidx_value = get_ith_agtype_value_from_container( - &(AG_GET_ARG_AGTYPE_P(2))->root, 0); + agt_uidx = AG_GET_ARG_AGTYPE_P(2); + uidx_value = get_ith_agtype_value_from_container(&agt_uidx->root, 0); /* adjust for AGTV_NULL */ if (uidx_value->type == AGTV_NULL) { @@ -4068,10 +4202,12 @@ Datum agtype_access_slice(PG_FUNCTION_ARGS) if (lidx_value) { lower_index = lidx_value->val.int_value; + pfree_agtype_value(lidx_value); } if (uidx_value) { upper_index = uidx_value->val.int_value; + pfree_agtype_value(uidx_value); } /* adjust for negative and out of bounds index values */ @@ -4127,7 +4263,14 @@ Datum agtype_access_slice(PG_FUNCTION_ARGS) result.res = push_agtype_value(&result.parse_state, WAGT_END_ARRAY, NULL); - PG_RETURN_POINTER(agtype_value_to_agtype(result.res)); + agt_result = agtype_value_to_agtype(result.res); + + pfree_agtype_in_state(&result); + PG_FREE_IF_COPY(agt_array, 0); + PG_FREE_IF_COPY(agt_lidx, 1); + PG_FREE_IF_COPY(agt_uidx, 2); + + PG_RETURN_POINTER(agt_result); } PG_FUNCTION_INFO_V1(agtype_in_operator); @@ -4286,6 +4429,7 @@ Datum agtype_string_match_starts_with(PG_FUNCTION_ARGS) { agtype *lhs = AG_GET_ARG_AGTYPE_P(0); agtype *rhs = AG_GET_ARG_AGTYPE_P(1); + bool result = false; if (AGT_ROOT_IS_SCALAR(lhs) && AGT_ROOT_IS_SCALAR(rhs)) { @@ -4298,17 +4442,33 @@ Datum agtype_string_match_starts_with(PG_FUNCTION_ARGS) if (lhs_value->type == AGTV_STRING && rhs_value->type == AGTV_STRING) { if (lhs_value->val.string.len < rhs_value->val.string.len) - return boolean_to_agtype(false); - - if (strncmp(lhs_value->val.string.val, rhs_value->val.string.val, - rhs_value->val.string.len) == 0) - return boolean_to_agtype(true); + { + result = false; + } + else if (strncmp(lhs_value->val.string.val, + rhs_value->val.string.val, + rhs_value->val.string.len) == 0) + { + result = true; + } else - return boolean_to_agtype(false); + { + result = false; + } } + pfree_agtype_value(lhs_value); + pfree_agtype_value(rhs_value); } - ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("agtype string values expected"))); + else + { + ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("agtype string values expected"))); + } + + PG_FREE_IF_COPY(lhs, 0); + PG_FREE_IF_COPY(rhs, 1); + + return boolean_to_agtype(result); } PG_FUNCTION_INFO_V1(agtype_string_match_ends_with); @@ -4319,6 +4479,7 @@ Datum agtype_string_match_ends_with(PG_FUNCTION_ARGS) { agtype *lhs = AG_GET_ARG_AGTYPE_P(0); agtype *rhs = AG_GET_ARG_AGTYPE_P(1); + bool result = false; if (AGT_ROOT_IS_SCALAR(lhs) && AGT_ROOT_IS_SCALAR(rhs)) { @@ -4331,19 +4492,35 @@ Datum agtype_string_match_ends_with(PG_FUNCTION_ARGS) if (lhs_value->type == AGTV_STRING && rhs_value->type == AGTV_STRING) { if (lhs_value->val.string.len < rhs_value->val.string.len) - return boolean_to_agtype(false); - - if (strncmp(lhs_value->val.string.val + lhs_value->val.string.len - - rhs_value->val.string.len, - rhs_value->val.string.val, - rhs_value->val.string.len) == 0) - return boolean_to_agtype(true); + { + result = false; + } + else if (strncmp((lhs_value->val.string.val + + lhs_value->val.string.len - + rhs_value->val.string.len), + rhs_value->val.string.val, + rhs_value->val.string.len) == 0) + { + result = true; + } else - return boolean_to_agtype(false); + { + result = false; + } } + pfree_agtype_value(lhs_value); + pfree_agtype_value(rhs_value); } - ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("agtype string values expected"))); + else + { + ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("agtype string values expected"))); + } + + PG_FREE_IF_COPY(lhs, 0); + PG_FREE_IF_COPY(rhs, 1); + + return boolean_to_agtype(result); } PG_FUNCTION_INFO_V1(agtype_string_match_contains); @@ -4354,6 +4531,7 @@ Datum agtype_string_match_contains(PG_FUNCTION_ARGS) { agtype *lhs = AG_GET_ARG_AGTYPE_P(0); agtype *rhs = AG_GET_ARG_AGTYPE_P(1); + bool result = false; if (AGT_ROOT_IS_SCALAR(lhs) && AGT_ROOT_IS_SCALAR(rhs)) { @@ -4369,19 +4547,37 @@ Datum agtype_string_match_contains(PG_FUNCTION_ARGS) char *r; if (lhs_value->val.string.len < rhs_value->val.string.len) - return boolean_to_agtype(false); + { + result = false; + } l = pnstrdup(lhs_value->val.string.val, lhs_value->val.string.len); r = pnstrdup(rhs_value->val.string.val, rhs_value->val.string.len); if (strstr(l, r) == NULL) - return boolean_to_agtype(false); + { + result = false; + } else - return boolean_to_agtype(true); + { + result = true; + } + pfree(l); + pfree(r); } + pfree_agtype_value(lhs_value); + pfree_agtype_value(rhs_value); } - ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("agtype string values expected"))); + else + { + ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("agtype string values expected"))); + } + + PG_FREE_IF_COPY(lhs, 0); + PG_FREE_IF_COPY(rhs, 1); + + return boolean_to_agtype(result); } #define LEFT_ROTATE(n, i) ((n << i) | (n >> (64 - i))) @@ -4423,7 +4619,10 @@ Datum agtype_hash_cmp(PG_FUNCTION_ARGS) seed = LEFT_ROTATE(seed, 1); } - PG_RETURN_INT16(hash); + pfree(r); + PG_FREE_IF_COPY(agt, 0); + + PG_RETURN_INT32(hash); } /* Comparison function for btree Indexes */ @@ -4471,23 +4670,31 @@ Datum agtype_typecast_numeric(PG_FUNCTION_ARGS) agtype_value result_value; Datum numd; char *string = NULL; + agtype *result = NULL; /* get the agtype equivalence of any convertable input type */ arg_agt = get_one_agtype_from_variadic_args(fcinfo, 0, 1); /* Return null if arg_agt is null. This covers SQL and Agtype NULLS */ if (arg_agt == NULL) + { PG_RETURN_NULL(); + } /* check that we have a scalar value */ if (!AGT_ROOT_IS_SCALAR(arg_agt)) + { ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("typecast argument must resolve to a scalar value"))); + } /* get the arg parameter */ arg_value = get_ith_agtype_value_from_container(&arg_agt->root, 0); + /* we don't need to agtype arg anymore */ + PG_FREE_IF_COPY(arg_agt, 0); + /* the input type drives the casting */ switch(arg_value->type) { @@ -4501,7 +4708,9 @@ Datum agtype_typecast_numeric(PG_FUNCTION_ARGS) break; case AGTV_NUMERIC: /* it is already a numeric so just return it */ - PG_RETURN_POINTER(agtype_value_to_agtype(arg_value)); + result = agtype_value_to_agtype(arg_value); + pfree_agtype_value(arg_value); + PG_RETURN_POINTER(result); break; /* this allows string numbers and NaN */ case AGTV_STRING: @@ -4527,11 +4736,16 @@ Datum agtype_typecast_numeric(PG_FUNCTION_ARGS) break; } + pfree_agtype_value(arg_value); + /* fill in and return our result */ result_value.type = AGTV_NUMERIC; result_value.val.numeric = DatumGetNumeric(numd); - PG_RETURN_POINTER(agtype_value_to_agtype(&result_value)); + result = agtype_value_to_agtype(&result_value); + pfree_agtype_value_content(&result_value); + + PG_RETURN_POINTER(result); } PG_FUNCTION_INFO_V1(agtype_typecast_int); @@ -10036,6 +10250,9 @@ agtype *get_one_agtype_from_variadic_args(FunctionCallInfo fcinfo, /* if null, return null */ if (nulls[0]) { + pfree(args); + pfree(nulls); + pfree(types); return NULL; } @@ -10054,6 +10271,11 @@ agtype *get_one_agtype_from_variadic_args(FunctionCallInfo fcinfo, if (AGTYPE_CONTAINER_IS_SCALAR(agtc) && AGTE_IS_NULL(agtc->children[0])) { + PG_FREE_IF_COPY(agtype_result, variadic_offset); + + pfree(args); + pfree(nulls); + pfree(types); return NULL; } } @@ -10073,7 +10295,14 @@ agtype *get_one_agtype_from_variadic_args(FunctionCallInfo fcinfo, datum_to_agtype(args[0], false, &state, tcategory, outfuncoid, false); /* convert it to an agtype */ agtype_result = agtype_value_to_agtype(state.res); + + pfree_agtype_in_state(&state); } + + pfree(args); + pfree(nulls); + pfree(types); + return agtype_result; } diff --git a/src/backend/utils/adt/agtype_util.c b/src/backend/utils/adt/agtype_util.c index 9982efc6b..5ba6e7278 100644 --- a/src/backend/utils/adt/agtype_util.c +++ b/src/backend/utils/adt/agtype_util.c @@ -87,7 +87,6 @@ static agtype_value *push_agtype_value_scalar(agtype_parse_state **pstate, agtype_value *scalar_val); static int compare_two_floats_orderability(float8 lhs, float8 rhs); static int get_type_sort_priority(enum agtype_value_type type); -static void pfree_agtype_value_content(agtype_value* value); static void pfree_iterator_agtype_value_token(agtype_iterator_token token, agtype_value *agtv); @@ -2365,7 +2364,7 @@ void pfree_agtype_value(agtype_value* value) * of the passed agtype_value only. It does not deallocate * `value` itself. */ -static void pfree_agtype_value_content(agtype_value* value) +void pfree_agtype_value_content(agtype_value* value) { int i; diff --git a/src/include/utils/agtype.h b/src/include/utils/agtype.h index 9cb758ec8..96b2300e2 100644 --- a/src/include/utils/agtype.h +++ b/src/include/utils/agtype.h @@ -552,6 +552,7 @@ agtype_iterator *get_next_list_element(agtype_iterator *it, agtype_container *agtc, agtype_value *elem); void pfree_agtype_value(agtype_value* value); +void pfree_agtype_value_content(agtype_value* value); void pfree_agtype_in_state(agtype_in_state* value); agtype_value *agtype_value_from_cstring(char *str, int len);