This is an automated email from the ASF dual-hosted git repository.

rafsun42 pushed a commit to branch PG13
in repository https://gitbox.apache.org/repos/asf/age.git


The following commit(s) were added to refs/heads/PG13 by this push:
     new 11819471 Clean up agtype_to_int8, agtype_to_int4, & agtype_to_int2 
(#1354) (#1370)
11819471 is described below

commit 11819471d5680c1f5b8710a5dfeaf8bbdec68184
Author: John Gemignani <[email protected]>
AuthorDate: Wed Nov 8 08:55:48 2023 -0800

    Clean up agtype_to_int8, agtype_to_int4, & agtype_to_int2 (#1354) (#1370)
    
    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 cb4063b0..d6e70f1c 100644
--- a/regress/expected/agtype.out
+++ b/regress/expected/agtype.out
@@ -2302,6 +2302,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
 --
@@ -2432,6 +2436,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
 --
@@ -2562,6 +2570,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 74eb4f3a..f7a905af 100644
--- a/regress/sql/agtype.sql
+++ b/regress/sql/agtype.sql
@@ -574,6 +574,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
@@ -609,7 +611,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
 --
@@ -644,6 +647,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 3e21ca9f..55944ddd 100644
--- a/src/backend/utils/adt/agtype.c
+++ b/src/backend/utils/adt/agtype.c
@@ -2651,14 +2651,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 */
@@ -2666,6 +2673,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 ||
@@ -2673,6 +2681,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 */
@@ -2696,7 +2709,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 */
@@ -2755,14 +2771,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 */
@@ -2770,6 +2793,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 ||
@@ -2777,6 +2801,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 */
@@ -2795,18 +2824,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 */
@@ -2865,14 +2892,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 */
@@ -2880,6 +2914,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 ||
@@ -2887,6 +2922,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 */
@@ -2905,18 +2945,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 */

Reply via email to