> On Sep 22, 2026, at 15:47, Peter Eisentraut <[email protected]> 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@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/
