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

On 22/09/2026 10:47, Peter Eisentraut 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.

+1, looks good to me.

- Heikki

On 22 Sep 2026, at 10:00, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

On 22/09/2026 10:47, Peter Eisentraut 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.

+1, looks good to me.

Agreed, that '1' has confused me more than once so glad to see it cleaned up.

--
Daniel Gustafsson

On Sep 22, 2026, at 15:47, Peter Eisentraut <peter@eisentraut.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@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/

On 22.09.26 11:09, Chao Li wrote:

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.
```

Thanks, committed with those adjustments.