This is an automated email from the ASF dual-hosted git repository. jgemignani pushed a commit to branch PG15 in repository https://gitbox.apache.org/repos/asf/age.git
commit 5b057f1c1a66c3a2def5e0901475e4b95d419ccb Author: John Gemignani <[email protected]> AuthorDate: Mon Nov 6 09:59:00 2023 -0800 Clean up agtype_to_int8, agtype_to_int4, & agtype_to_int2 (#1354) Cleaned up the code in `agtype_to_int8`, `agtype_to_int4`, and `agtype_to_int2`. There was some dead code after implementing PR 1339. Additionally, modified the error messages to be a bit more helpful and modified the logic to cover a few more cases. Added more regression tests. --- regress/expected/agtype.out | 12 +++++++ regress/sql/agtype.sql | 7 +++- src/backend/utils/adt/agtype.c | 77 +++++++++++++++++++++++++++++++----------- 3 files changed, 76 insertions(+), 20 deletions(-) diff --git a/regress/expected/agtype.out b/regress/expected/agtype.out index 53217f67..66344c6e 100644 --- a/regress/expected/agtype.out +++ b/regress/expected/agtype.out @@ -2829,6 +2829,10 @@ SELECT agtype_to_int8(Inf); ERROR: column "inf" does not exist LINE 1: SELECT agtype_to_int8(Inf); ^ +SELECT agtype_to_int8('{"name":"John"}'); +ERROR: invalid agtype string to int8 type: 17 +SELECT agtype_to_int8('[1,2,3]'); +ERROR: invalid agtype string to int8 type: 16 -- -- Test boolean to integer cast -- @@ -2959,6 +2963,10 @@ SELECT agtype_to_int4(Inf); ERROR: column "inf" does not exist LINE 1: SELECT agtype_to_int4(Inf); ^ +SELECT agtype_to_int4('{"name":"John"}'); +ERROR: invalid agtype string to int4 type: 17 +SELECT agtype_to_int4('[1,2,3]'); +ERROR: invalid agtype string to int4 type: 16 -- -- Test boolean to integer2 cast -- @@ -3089,6 +3097,10 @@ SELECT agtype_to_int2(Inf); ERROR: column "inf" does not exist LINE 1: SELECT agtype_to_int2(Inf); ^ +SELECT agtype_to_int2('{"name":"John"}'); +ERROR: invalid agtype string to int2 type: 17 +SELECT agtype_to_int2('[1,2,3]'); +ERROR: invalid agtype string to int2 type: 16 -- -- Test agtype to int[] -- diff --git a/regress/sql/agtype.sql b/regress/sql/agtype.sql index c17290af..4089d524 100644 --- a/regress/sql/agtype.sql +++ b/regress/sql/agtype.sql @@ -693,6 +693,8 @@ SELECT agtype_to_int8('NaN'); SELECT agtype_to_int8('Inf'); SELECT agtype_to_int8(NaN); SELECT agtype_to_int8(Inf); +SELECT agtype_to_int8('{"name":"John"}'); +SELECT agtype_to_int8('[1,2,3]'); -- -- Test boolean to integer cast @@ -728,7 +730,8 @@ SELECT agtype_to_int4('NaN'); SELECT agtype_to_int4('Inf'); SELECT agtype_to_int4(NaN); SELECT agtype_to_int4(Inf); - +SELECT agtype_to_int4('{"name":"John"}'); +SELECT agtype_to_int4('[1,2,3]'); -- -- Test boolean to integer2 cast -- @@ -763,6 +766,8 @@ SELECT agtype_to_int2('NaN'); SELECT agtype_to_int2('Inf'); SELECT agtype_to_int2(NaN); SELECT agtype_to_int2(Inf); +SELECT agtype_to_int2('{"name":"John"}'); +SELECT agtype_to_int2('[1,2,3]'); -- -- Test agtype to int[] diff --git a/src/backend/utils/adt/agtype.c b/src/backend/utils/adt/agtype.c index 2914b52c..c73d066f 100644 --- a/src/backend/utils/adt/agtype.c +++ b/src/backend/utils/adt/agtype.c @@ -2663,14 +2663,21 @@ Datum agtype_to_int8(PG_FUNCTION_ARGS) { agtype_value *temp = NULL; - /* convert the string to an agtype_value */ + /* + * Convert the string to an agtype_value. Remember that a returned + * scalar value is returned in a one element array. + */ temp = agtype_value_from_cstring(agtv_p->val.string.val, agtv_p->val.string.len); + /* this will catch anything that isn't an array and isn't a scalar */ if (temp->type != AGTV_ARRAY || !temp->val.array.raw_scalar) { - elog(ERROR, "invalid agtype type: %d", (int)agtv_p->type); + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid agtype string to int8 type: %d", + (int)temp->type))); } /* save the top agtype_value */ @@ -2678,6 +2685,7 @@ Datum agtype_to_int8(PG_FUNCTION_ARGS) /* get the wrapped agtype_value */ temp = &temp->val.array.elems[0]; + /* these we expect */ if (temp->type == AGTV_FLOAT || temp->type == AGTV_INTEGER || temp->type == AGTV_NUMERIC || @@ -2685,6 +2693,11 @@ Datum agtype_to_int8(PG_FUNCTION_ARGS) { agtv_p = temp; } + else + { + elog(ERROR, "unexpected string type: %d in agtype_to_int8", + (int)temp->type); + } } /* now check the rest */ @@ -2708,7 +2721,10 @@ Datum agtype_to_int8(PG_FUNCTION_ARGS) } else { - elog(ERROR, "invalid agtype type: %d", (int)agtv_p->type); + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid conversion type in agtype_to_int8: %d", + (int)agtv_p->type))); } /* free the container, if it was used */ @@ -2767,14 +2783,21 @@ Datum agtype_to_int4(PG_FUNCTION_ARGS) { agtype_value *temp = NULL; - /* convert the string to an agtype_value */ + /* + * Convert the string to an agtype_value. Remember that a returned + * scalar value is returned in a one element array. + */ temp = agtype_value_from_cstring(agtv_p->val.string.val, agtv_p->val.string.len); + /* this will catch anything that isn't an array and isn't a scalar */ if (temp->type != AGTV_ARRAY || !temp->val.array.raw_scalar) { - elog(ERROR, "invalid agtype type: %d", (int)agtv_p->type); + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid agtype string to int4 type: %d", + (int)temp->type))); } /* save the top agtype_value */ @@ -2782,6 +2805,7 @@ Datum agtype_to_int4(PG_FUNCTION_ARGS) /* get the wrapped agtype_value */ temp = &temp->val.array.elems[0]; + /* these we expect */ if (temp->type == AGTV_FLOAT || temp->type == AGTV_INTEGER || temp->type == AGTV_NUMERIC || @@ -2789,6 +2813,11 @@ Datum agtype_to_int4(PG_FUNCTION_ARGS) { agtv_p = temp; } + else + { + elog(ERROR, "unexpected string type: %d in agtype_to_int4", + (int)temp->type); + } } /* now check the rest */ @@ -2807,18 +2836,16 @@ Datum agtype_to_int4(PG_FUNCTION_ARGS) result = DatumGetInt32(DirectFunctionCall1(numeric_int4, NumericGetDatum(agtv_p->val.numeric))); } - else if (agtv_p->type == AGTV_STRING) - { - result = DatumGetInt32(DirectFunctionCall1(int4in, - CStringGetDatum(agtv_p->val.string.val))); - } else if (agtv_p->type == AGTV_BOOL) { result = (agtv_p->val.boolean) ? 1 : 0; } else { - elog(ERROR, "invalid agtype type: %d", (int)agtv_p->type); + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid conversion type in agtype_to_int4: %d", + (int)agtv_p->type))); } /* free the container, if it was used */ @@ -2877,14 +2904,21 @@ Datum agtype_to_int2(PG_FUNCTION_ARGS) { agtype_value *temp = NULL; - /* convert the string to an agtype_value */ + /* + * Convert the string to an agtype_value. Remember that a returned + * scalar value is returned in a one element array. + */ temp = agtype_value_from_cstring(agtv_p->val.string.val, agtv_p->val.string.len); + /* this will catch anything that isn't an array and isn't a scalar */ if (temp->type != AGTV_ARRAY || !temp->val.array.raw_scalar) { - elog(ERROR, "invalid agtype type: %d", (int)agtv_p->type); + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid agtype string to int2 type: %d", + (int)temp->type))); } /* save the top agtype_value */ @@ -2892,6 +2926,7 @@ Datum agtype_to_int2(PG_FUNCTION_ARGS) /* get the wrapped agtype_value */ temp = &temp->val.array.elems[0]; + /* these we expect */ if (temp->type == AGTV_FLOAT || temp->type == AGTV_INTEGER || temp->type == AGTV_NUMERIC || @@ -2899,6 +2934,11 @@ Datum agtype_to_int2(PG_FUNCTION_ARGS) { agtv_p = temp; } + else + { + elog(ERROR, "unexpected string type: %d in agtype_to_int2", + (int)temp->type); + } } /* now check the rest */ @@ -2917,18 +2957,17 @@ Datum agtype_to_int2(PG_FUNCTION_ARGS) result = DatumGetInt16(DirectFunctionCall1(numeric_int2, NumericGetDatum(agtv_p->val.numeric))); } - else if (agtv_p->type == AGTV_STRING) - { - result = DatumGetInt16(DirectFunctionCall1(int2in, - CStringGetDatum(agtv_p->val.string.val))); - } else if (agtv_p->type == AGTV_BOOL) { result = (agtv_p->val.boolean) ? 1 : 0; } else { - elog(ERROR, "invalid agtype type: %d", (int)agtv_p->type); + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid conversion type in agtype_to_int2: %d", + (int)agtv_p->type))); + } /* free the container, if it was used */
