Skip to content

Commit

Permalink
Clean up agtype_to_int8, agtype_to_int4, & agtype_to_int2 (apache#1354)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jrgemignani
jrgemignani committed Nov 7, 2023
1 parent 1f6560e commit 1893db5
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 20 deletions.
12 changes: 12 additions & 0 deletions regress/expected/agtype.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2301,6 +2301,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
--
Expand Down Expand Up @@ -2431,6 +2435,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
--
Expand Down Expand Up @@ -2561,6 +2569,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[]
--
Expand Down
7 changes: 6 additions & 1 deletion regress/sql/agtype.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,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
Expand Down Expand Up @@ -608,7 +610,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
--
Expand Down Expand Up @@ -643,6 +646,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[]
Expand Down
77 changes: 58 additions & 19 deletions src/backend/utils/adt/agtype.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2629,28 +2629,41 @@ 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 */
container = temp;
/* 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 ||
temp->type == AGTV_BOOL)
{
agtv_p = temp;
}
else
{
elog(ERROR, "unexpected string type: %d in agtype_to_int8",
(int)temp->type);
}
}

/* now check the rest */
Expand All @@ -2674,7 +2687,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 */
Expand Down Expand Up @@ -2733,28 +2749,41 @@ 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 */
container = temp;
/* 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 ||
temp->type == AGTV_BOOL)
{
agtv_p = temp;
}
else
{
elog(ERROR, "unexpected string type: %d in agtype_to_int4",
(int)temp->type);
}
}

/* now check the rest */
Expand All @@ -2773,18 +2802,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 */
Expand Down Expand Up @@ -2843,28 +2870,41 @@ 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 */
container = temp;
/* 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 ||
temp->type == AGTV_BOOL)
{
agtv_p = temp;
}
else
{
elog(ERROR, "unexpected string type: %d in agtype_to_int2",
(int)temp->type);
}
}

/* now check the rest */
Expand All @@ -2883,18 +2923,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 */
Expand Down

0 comments on commit 1893db5

Please sign in to comment.