Re: Declare variable-length catalog columns as [] rather than [1]

> On Sep 22, 2026, at 15:47, Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> Variable-length catalog columns have been declared like
>
> text attoptions[1];
>
> but that "1" has always been a fiction. Before the use of #ifdef CATALOG_VARLEN, these declarations were visible to the C compiler, and this was also before flexible array members were universally available, so this was just a convenient workaround to make this compile. But these reasons are long gone, and the "1" is now just a confusing relic. Change this to
>
> text attoptions[];
>
> which more intuitively reflects the actual nature of these fields (while still being syntactically valid but semantically invalid C code).
>
> Catalog.pm could already parse both spellings, but no existing code used bare []. To enforce future consistency, it is changed to no longer permit digits between the brackets.
>
> (Obviously, catalog definitions are not backpatched, so this shouldn't create any new maintenance burden.)
> <0001-Declare-variable-length-catalog-columns-as-rather-th.patch>

I agree that changing [1] to [] is clearer.

After applying the patch, I changed one attribute back from [] to [1]. The build then produced a lot of warnings, for example:
```
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Werror=unguarded-availability-new -Wmissing-format-attribute -Wimplicit-fallthrough -Wcast-function-type -Wformat-security -Wmissing-variable-declarations -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-unused-command-line-argument -Wno-compound-token-split-by-macro -Wno-format-truncation -Wno-cast-function-type-strict -O1 -g -fsanitize=address -fno-omit-frame-pointer -Wstrict-prototypes -Wold-style-definition -I../../../src/include -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX26.5.sdk -I/opt/homebrew/opt/icu4c(at)78/include -c -o catalog.o catalog.c
In file included from catalog.c:24:
In file included from ../../../src/include/access/htup_details.h:19:
In file included from ../../../src/include/access/tupdesc.h:18:
In file included from ../../../src/include/catalog/pg_attribute.h:26:
../../../src/include/catalog/pg_attribute_d.h:50:37: warning: ISO C99 requires whitespace after the macro name [-Wc99-extensions]
50 | #define Anum_pg_attribute_attoptions[1] 23
| ^
1 warning generated.
```

I think this comes from the change in Catalog.pm:
```
- # If the C name ends with '[]' or '[digits]', we have
- # an array type, so we discard that from the name and
- # prepend '_' to the type.
- if ($attname =~ /(\w+)\[\d*\]/)
+ # If the C name ends with '[]', we have an array type,
+ # so we discard that from the name and prepend '_' to
+ # the type.
+ if ($attname =~ /(\w+)\[\]/)
{
```

After this change, a declaration using [1] is no longer recognized as an array, so the [1] remains part of attname and eventually gets copied into generated identifiers such as Anum_pg_attribute_attoptions[1].

Since the intention of this change is to disallow the old [digits] spelling, I think it would be better to detect it explicitly and fail with a clear error, for example:
```
# If the C name ends with '[]', we have an array type,
# so we discard that from the name and prepend '_' to
# the type.
if ($attname =~ /(\w+)\[\]/)
{
$attname = $1;
$atttype = '_' . $atttype;
}
elsif ($attname =~ /\w+\[\d+\]/)
{
die "catalog array column '$atttype $attname' must be declared with []";
}
```

This way, the build fails immediately with a clearer error:
```
catalog array column 'text attoptions[1]' must be declared with [] at /Users/chaol/Documents/code/postgresql/src/backend/catalog/Catalog.pm line 239, <$ifh> line 177.
```

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