From: Chao Li <li.evan.chao@gmail.com>
Message-Id: <63229AD5-48DB-417C-9361-EA478DAF57AF@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_6C9AB3F7-0AC0-4394-8391-199FDD3D7BC0"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: guc: make dereference style consistent in
 check_backtrace_functions
Date: Fri, 27 Feb 2026 09:33:45 +0800
In-Reply-To: <202602261236.ttanikrvfx6w@alvherre.pgsql>
Cc: Junwang Zhao <zhjwpku@gmail.com>,
 zhanghu <kongbaik228@gmail.com>,
 pgsql-hackers@lists.postgresql.org
To: =?utf-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>
References: <202602261236.ttanikrvfx6w@alvherre.pgsql>
Archived-At: <https://www.postgresql.org/message-id/63229AD5-48DB-417C-9361-EA478DAF57AF%40gmail.com>
Precedence: bulk


--Apple-Mail=_6C9AB3F7-0AC0-4394-8391-199FDD3D7BC0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8


> On Feb 26, 2026, at 20:37, =C3=81lvaro Herrera <alvherre@kurilemu.de> =
wrote:
>=20
> There is at least one more place in the code where this is done.
>=20

I did a search with the command: grep -RInE =
'\*[[:space:]]*[A-Za-z_][A-Za-z0-9_]*\[0\]' src contrib --include=3D'*.c'

Excluding irrelevant results, there are 3 more occurrences:

1 - contrib/basic_archive/basic_archive.c line 105
```
	if (*newval =3D=3D NULL || *newval[0] =3D=3D '\0')
		return true;
```

Here, the code checks *newval first, which implies that the subsequent =
*newval[0] is unintentional syntax.

2 - src/interfaces/ecpg/pgtypeslib/interval.c line 62
```
int
DecodeInterval(char **field, int *ftype, int nf,	/* int range, */
			   int *dtype, struct /* pg_ */ tm *tm, fsec_t =
*fsec)
{
  ...
	if (IntervalStyle =3D=3D INTSTYLE_SQL_STANDARD && *field[0] =3D=3D=
 '-')
	{
		/* Check for additional explicit signs */
		bool		more_signs =3D false;

		for (i =3D 1; i < nf; i++)
		{
			if (*field[i] =3D=3D '-' || *field[i] =3D=3D =
'+')
			{
				more_signs =3D true;
				break;
			}
		}
```

3 - src/backend/utils/adt/datatime.c line 3522
```
int
DecodeInterval(char **field, int *ftype, int nf, int range,
			   int *dtype, struct pg_itm_in *itm_in)
{
 ...
	if (IntervalStyle =3D=3D INTSTYLE_SQL_STANDARD && nf > 0 && =
*field[0] =3D=3D '-')
	{
		force_negative =3D true;
		/* Check for additional explicit signs */
		for (i =3D 1; i < nf; i++)
		{
			if (*field[i] =3D=3D '-' || *field[i] =3D=3D =
'+')
			{
				force_negative =3D false;
				break;
			}
		}
	}
```

Where 2&3 makes this patch more interesting.

Both occurrences are inside functions named DecodeInterval. For non-zero =
i, the code also performs *field[i]:

Given this code has been there for years, I don=E2=80=99t believe it is =
a bug. I checked the callers of DecodeInterval in both files and found =
that field is defined as:
```
    char *field[MAXDATEFIELDS];
```

This explains why *field[i] works; it is doing the intended thing by =
getting the first character of the string at array position i.

However, since the precedence between the [] and * operators frequently =
confuses people, I suggest adding parentheses to make the intention =
explicit as *(field[i]). Furthermore, I think we should change the =
function signatures to use the type char *field[] to reflect the actual =
type the functions expect. If a caller were to pass a true char ** typed =
field to DecodeInterval, the current logic would result in a bug.

See the attached diff for my suggested changes.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/


--Apple-Mail=_6C9AB3F7-0AC0-4394-8391-199FDD3D7BC0
Content-Disposition: attachment;
	filename=nocfbot.diff
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="nocfbot.diff"
Content-Transfer-Encoding: 7bit

diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 90946db72ff..7d845700163 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -3483,7 +3483,7 @@ ClearPgItmIn(struct pg_itm_in *itm_in)
  * suffices.
  */
 int
-DecodeInterval(char **field, int *ftype, int nf, int range,
+DecodeInterval(char *field[], int *ftype, int nf, int range,
 			   int *dtype, struct pg_itm_in *itm_in)
 {
 	bool		force_negative = false;
@@ -3519,13 +3519,13 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 	 * to dump in postgres style, not SQL style.)
 	 *----------
 	 */
-	if (IntervalStyle == INTSTYLE_SQL_STANDARD && nf > 0 && *field[0] == '-')
+	if (IntervalStyle == INTSTYLE_SQL_STANDARD && nf > 0 && *(field[0]) == '-')
 	{
 		force_negative = true;
 		/* Check for additional explicit signs */
 		for (i = 1; i < nf; i++)
 		{
-			if (*field[i] == '-' || *field[i] == '+')
+			if (*(field[i]) == '-' || *(field[i]) == '+')
 			{
 				force_negative = false;
 				break;
@@ -3557,7 +3557,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 				 * least one digit; there could be ':', '.', '-' embedded in
 				 * it as well.
 				 */
-				Assert(*field[i] == '-' || *field[i] == '+');
+				Assert(*(field[i]) == '-' || *(field[i]) == '+');
 
 				/*
 				 * Check for signed hh:mm or hh:mm:ss.  If so, process exactly
@@ -3567,7 +3567,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 					DecodeTimeForInterval(field[i] + 1, fmask, range,
 										  &tmask, itm_in) == 0)
 				{
-					if (*field[i] == '-')
+					if (*(field[i]) == '-')
 					{
 						/* flip the sign on time field */
 						if (itm_in->tm_usec == PG_INT64_MIN)
@@ -3650,7 +3650,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 					if (*cp != '\0')
 						return DTERR_BAD_FORMAT;
 					type = DTK_MONTH;
-					if (*field[i] == '-')
+					if (*(field[i]) == '-')
 						val2 = -val2;
 					if (pg_mul_s64_overflow(val, MONTHS_PER_YEAR, &val))
 						return DTERR_FIELD_OVERFLOW;
@@ -3663,7 +3663,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 					dterr = ParseFraction(cp, &fval);
 					if (dterr)
 						return dterr;
-					if (*field[i] == '-')
+					if (*(field[i]) == '-')
 						fval = -fval;
 				}
 				else if (*cp == '\0')
diff --git a/src/include/utils/datetime.h b/src/include/utils/datetime.h
index f77c6acd8b6..66dd871fd7f 100644
--- a/src/include/utils/datetime.h
+++ b/src/include/utils/datetime.h
@@ -316,7 +316,7 @@ extern int	DecodeTimezone(const char *str, int *tzp);
 extern int	DecodeTimeOnly(char **field, int *ftype, int nf,
 						   int *dtype, struct pg_tm *tm, fsec_t *fsec, int *tzp,
 						   DateTimeErrorExtra *extra);
-extern int	DecodeInterval(char **field, int *ftype, int nf, int range,
+extern int	DecodeInterval(char *field[], int *ftype, int nf, int range,
 						   int *dtype, struct pg_itm_in *itm_in);
 extern int	DecodeISO8601Interval(char *str,
 								  int *dtype, struct pg_itm_in *itm_in);
diff --git a/src/interfaces/ecpg/pgtypeslib/dt.h b/src/interfaces/ecpg/pgtypeslib/dt.h
index 00a45799d55..47df0df4739 100644
--- a/src/interfaces/ecpg/pgtypeslib/dt.h
+++ b/src/interfaces/ecpg/pgtypeslib/dt.h
@@ -311,7 +311,7 @@ do { \
 #define TIMESTAMP_IS_NOEND(j)	((j) == DT_NOEND)
 #define TIMESTAMP_NOT_FINITE(j) (TIMESTAMP_IS_NOBEGIN(j) || TIMESTAMP_IS_NOEND(j))
 
-int			DecodeInterval(char **field, int *ftype, int nf, int *dtype, struct tm *tm, fsec_t *fsec);
+int			DecodeInterval(char *field[], int *ftype, int nf, int *dtype, struct tm *tm, fsec_t *fsec);
 int			DecodeTime(char *str, int *tmask, struct tm *tm, fsec_t *fsec);
 void		EncodeDateTime(struct tm *tm, fsec_t fsec, bool print_tz, int tz, const char *tzn, int style, char *str, bool EuroDates);
 void		EncodeInterval(struct tm *tm, fsec_t fsec, int style, char *str);
diff --git a/src/interfaces/ecpg/pgtypeslib/interval.c b/src/interfaces/ecpg/pgtypeslib/interval.c
index e452a088f9e..4d0e75d4dd4 100644
--- a/src/interfaces/ecpg/pgtypeslib/interval.c
+++ b/src/interfaces/ecpg/pgtypeslib/interval.c
@@ -323,7 +323,7 @@ DecodeISO8601Interval(char *str,
  *		int IntervalStyle = INTSTYLE_POSTGRES;
  */
 int
-DecodeInterval(char **field, int *ftype, int nf,	/* int range, */
+DecodeInterval(char *field[], int *ftype, int nf,	/* int range, */
 			   int *dtype, struct /* pg_ */ tm *tm, fsec_t *fsec)
 {
 	int			IntervalStyle = INTSTYLE_POSTGRES_VERBOSE;
@@ -362,7 +362,7 @@ DecodeInterval(char **field, int *ftype, int nf,	/* int range, */
 				 * least one digit; there could be ':', '.', '-' embedded in
 				 * it as well.
 				 */
-				Assert(*field[i] == '-' || *field[i] == '+');
+				Assert(*(field[i]) == '-' || *(field[i]) == '+');
 
 				/*
 				 * Try for hh:mm or hh:mm:ss.  If not, fall through to
@@ -373,7 +373,7 @@ DecodeInterval(char **field, int *ftype, int nf,	/* int range, */
 					DecodeTime(field[i] + 1,	/* INTERVAL_FULL_RANGE, */
 							   &tmask, tm, fsec) == 0)
 				{
-					if (*field[i] == '-')
+					if (*(field[i]) == '-')
 					{
 						/* flip the sign on all fields */
 						tm->tm_hour = -tm->tm_hour;
@@ -447,7 +447,7 @@ DecodeInterval(char **field, int *ftype, int nf,	/* int range, */
 					if (*cp != '\0')
 						return DTERR_BAD_FORMAT;
 					type = DTK_MONTH;
-					if (*field[i] == '-')
+					if (*(field[i]) == '-')
 						val2 = -val2;
 					val = val * MONTHS_PER_YEAR + val2;
 					fval = 0;
@@ -459,7 +459,7 @@ DecodeInterval(char **field, int *ftype, int nf,	/* int range, */
 					if (*cp != '\0' || errno != 0)
 						return DTERR_BAD_FORMAT;
 
-					if (*field[i] == '-')
+					if (*(field[i]) == '-')
 						fval = -fval;
 				}
 				else if (*cp == '\0')
@@ -622,14 +622,14 @@ DecodeInterval(char **field, int *ftype, int nf,	/* int range, */
 	 * to dump in postgres style, not SQL style.)
 	 *----------
 	 */
-	if (IntervalStyle == INTSTYLE_SQL_STANDARD && *field[0] == '-')
+	if (IntervalStyle == INTSTYLE_SQL_STANDARD && *(field[0]) == '-')
 	{
 		/* Check for additional explicit signs */
 		bool		more_signs = false;
 
 		for (i = 1; i < nf; i++)
 		{
-			if (*field[i] == '-' || *field[i] == '+')
+			if (*(field[i]) == '-' || *(field[i]) == '+')
 			{
 				more_signs = true;
 				break;

--Apple-Mail=_6C9AB3F7-0AC0-4394-8391-199FDD3D7BC0--