> On Feb 26, 2026, at 20:37, Álvaro Herrera <alvherre@kurilemu.de> wrote: > > There is at least one more place in the code where this is done. >
I did a search with the command: grep -RInE '\*[[:space:]]*[A-Za-z_][A-Za-z0-9_]*\[0\]' src contrib --include='*.c'
Excluding irrelevant results, there are 3 more occurrences:
1 - contrib/basic_archive/basic_archive.c line 105 ``` if (*newval == NULL || *newval[0] == '\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 == 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] == '+') { more_signs = 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 == 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] == '+') { force_negative = 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’t 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.
Thank you all for the reviews and detailed feedback.
Álvaro, thanks for pointing out that there were additional occurrences elsewhere in the tree. I have updated the original patch to address those cases; the revised version is attached as v2-0001.
I also appreciate the review and suggestions from Chao and Junwang.
Regarding the additional changes suggested by Chao: they go somewhat beyond the original scope of my original patch. To keep the discussion concrete, I have included Chao’s proposed diff as a separate patch (v2-0002) so it can be reviewed independently.
I have reviewed v2-0002 locally, and it looks good to me.
