Re: Declare variable-length catalog columns as [] rather than [1] - Mailing list pgsql-hackers

From Chao Li
Subject Re: Declare variable-length catalog columns as [] rather than [1]
Date
Msg-id 617515F3-5921-4A83-8C94-C5270BD47EB1@gmail.com
Whole thread
In response to Declare variable-length catalog columns as [] rather than [1]  (Peter Eisentraut <peter@eisentraut.org>)
Responses Re: Declare variable-length catalog columns as [] rather than [1]
List pgsql-hackers

> 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
theC compiler, and this was also before flexible array members were universally available, so this was just a
convenientworkaround to make this compile.  But these reasons are long gone, and the "1" is now just a confusing relic.
Changethis to 
>
>    text attoptions[];
>
> which more intuitively reflects the actual nature of these fields (while still being syntactically valid but
semanticallyinvalid C code). 
>
> Catalog.pm could already parse both spellings, but no existing code used bare [].  To enforce future consistency, it
ischanged 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
eventuallygets 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
explicitlyand 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.pmline 239, <$ifh> line 177. 
```

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







pgsql-hackers by date:

Previous
From: solai v
Date:
Subject: Re: [PATCH] Remove unused PageIsPredicateLocked()
Next
From: Bertrand Drouvot
Date:
Subject: Re: Introduce XID age based replication slot invalidation