v10-0006-refactor-ReadDimensionInt.patch

text/x-patch

Filename: v10-0006-refactor-ReadDimensionInt.patch
Type: text/x-patch
Part: 5
Message: Re: Cleaning up array_in()

Patch

Format: format-patch
Series: patch v10-0006
Subject: refactor ReadDimensionInt.
File+
src/backend/utils/adt/arrayfuncs.c 21 30
src/test/regress/expected/arrays.out 6 0
src/test/regress/sql/arrays.sql 1 0
From ac290ac7f8bbcb7523f432fb684ec5791975262e Mon Sep 17 00:00:00 2001
From: pgaddict <jian.universality@gmail.com>
Date: Mon, 11 Sep 2023 12:17:55 +0800
Subject: [PATCH v9 6/7] refactor ReadDimensionInt.

in array_in, if bound info explicily mentioned, then check and validdate bound info
of a array.
in array_in initialize variable dim and lBound. we need use the variable defaults valus in function ReadArrayDimensions and ReadArrayStr.
---
 src/backend/utils/adt/arrayfuncs.c   | 51 ++++++++++++----------------
 src/test/regress/expected/arrays.out |  6 ++++
 src/test/regress/sql/arrays.sql      |  1 +
 3 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index a2d806e3..95a3d5f6 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -188,7 +188,6 @@ array_in(PG_FUNCTION_ARGS)
 	Oid			typioparam;
 	char	   *p;
 	int			nitems;
-	int			nitems_according_to_dims;
 	Datum	   *values;
 	bool	   *nulls;
 	bool		hasnulls;
@@ -232,7 +231,12 @@ array_in(PG_FUNCTION_ARGS)
 	typalign = my_extra->typalign;
 	typdelim = my_extra->typdelim;
 	typioparam = my_extra->typioparam;
-
+	/* initialize dim, lBound for ReadArrayDimensions, ReadArrayStr */
+	for (int i = 0; i < MAXDIM; i++)
+	{
+		dim[i] = -1;
+		lBound[i] = 1;
+	}
 	/*
 	 * Start processing the input string.
 	 *
@@ -245,14 +249,12 @@ array_in(PG_FUNCTION_ARGS)
 
 	if (ndim == 0)
 	{
-		/* No array dimensions, so intuit dimensions from brace structure */
+		/* No array dimensions, so first literal character should be an opening brace */
 		if (*p != '{')
 			ereturn(escontext, (Datum) 0,
 					(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
 					 errmsg("malformed array literal: \"%s\"", string),
 					 errdetail("Array value must start with \"{\" or dimension information.")));
-		for (int i = 0; i < MAXDIM; i++)
-			lBound[i] = 1;
 	}
 	else
 	{
@@ -304,28 +306,17 @@ array_in(PG_FUNCTION_ARGS)
 		initStringInfo(&buf);
 
 		appendStringInfo(&buf, "array_in- ndim %d (", ndim);
-		for (int i = 0; i < ndim; i++)
+		for (int i = 0; i < MAXDIM; i++)
 			appendStringInfo(&buf, " %d", dim[i]);
+		appendStringInfo(&buf, "lBound info");
+		for (int i = 0; i < MAXDIM; i++)
+			appendStringInfo(&buf, " %d", lBound[i]);
 		appendStringInfo(&buf, ") for %s\n", string);
 		elog(NOTICE, "%s", buf.data);
 		pfree(buf.data);
 	}
 #endif
 
-	/* This checks for overflow of the array dimensions */
-
-	/*
-	 * FIXME: Is this still required? I believe all the checks it performs are
-	 * redundant with other checks in ReadArrayDimension() and ReadArrayStr()
-	 */
-	nitems_according_to_dims = ArrayGetNItemsSafe(ndim, dim, escontext);
-	if (nitems_according_to_dims < 0)
-		PG_RETURN_NULL();
-	if (nitems != nitems_according_to_dims)
-		elog(ERROR, "mismatch nitems, %d vs %d", nitems, nitems_according_to_dims);
-	if (!ArrayCheckBoundsSafe(ndim, dim, lBound, escontext))
-		PG_RETURN_NULL();
-
 	/* Empty array? */
 	if (nitems == 0)
 		PG_RETURN_ARRAYTYPE_P(construct_empty_array(element_type));
@@ -485,8 +476,8 @@ ReadArrayDimensions(char **srcptr, int *ndim_p, int *dim, int *lBound,
 							   "]")));
 		p++;
 
-		/* Upper bound of INT_MAX is disallowed, cf ArrayCheckBounds() */
-		if (ub == INT_MAX)
+		/* Upper bound of PG_INT32_MAX is disallowed, cf ArrayCheckBounds() */
+		if (ub == PG_INT32_MAX)
 			ereturn(escontext, false,
 					(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 					 errmsg("array upper bound is too large: %d", ub)));
@@ -521,6 +512,7 @@ static bool
 ReadDimensionInt(char **srcptr, int *result, const char *origStr, Node *escontext)
 {
 	char	   *p = *srcptr;
+	long	l;
 
 	/* don't accept leading whitespace */
 	if (!isdigit((unsigned char) *p) && *p != '-' && *p != '+')
@@ -530,11 +522,16 @@ ReadDimensionInt(char **srcptr, int *result, const char *origStr, Node *escontex
 	}
 
 	errno = 0;
-	*result = strtol(p, srcptr, 10);
-	if (errno == ERANGE)
+	l = strtol(p, srcptr, 10);
+
+	if (errno == ERANGE || l > PG_INT32_MAX || l < PG_INT32_MIN)
+	{
 		ereturn(escontext, false,
 				(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 				 errmsg("array bound is out of range")));
+	}
+
+	*result = l;
 	return true;
 }
 
@@ -603,12 +600,6 @@ ReadArrayStr(char **srcptr,
 	/* The caller already checked this */
 	Assert(**srcptr == '{');
 
-	if (!dimensions_specified)
-	{
-		/* Initialize dim[] entries to -1 meaning "unknown" */
-		for (int i = 0; i < MAXDIM; ++i)
-			dim[i] = -1;
-	}
 	ndim_frozen = dimensions_specified;
 
 	maxitems = 16;
diff --git a/src/test/regress/expected/arrays.out b/src/test/regress/expected/arrays.out
index 93e11068..bea0d00b 100644
--- a/src/test/regress/expected/arrays.out
+++ b/src/test/regress/expected/arrays.out
@@ -1615,6 +1615,12 @@ select '[1:0]={}'::int[];
  {}
 (1 row)
 
+select '[-2147483648:-2147483647]={1,2}'::int[];
+              int4               
+---------------------------------
+ [-2147483648:-2147483647]={1,2}
+(1 row)
+
 -- all of the above should be accepted
 -- tests for array aggregates
 CREATE TEMP TABLE arraggtest ( f1 INT[], f2 TEXT[][], f3 FLOAT[]);
diff --git a/src/test/regress/sql/arrays.sql b/src/test/regress/sql/arrays.sql
index b7e2b180..5eacb351 100644
--- a/src/test/regress/sql/arrays.sql
+++ b/src/test/regress/sql/arrays.sql
@@ -485,6 +485,7 @@ select array[]::text[];
 select '[0:1]={1.1,2.2}'::float8[];
 select '[2147483646:2147483646]={1}'::int[];
 select '[1:0]={}'::int[];
+select '[-2147483648:-2147483647]={1,2}'::int[];
 -- all of the above should be accepted
 
 -- tests for array aggregates
-- 
2.34.1