Thread: catalog files simplification

catalog files simplification

From
Peter Eisentraut
Date:
The current catalog files all do this:

    CATALOG(pg_aggregate,2600,AggregateRelationId)
    {
        ...
    } FormData_pg_aggregate;

    typedef FormData_pg_aggregate *Form_pg_aggregate;

The bottom part of this seems redundant.  With the attached patch, we
can generate that automatically, so this becomes just

    CATALOG(pg_aggregate,2600,AggregateRelationId)
    {
        ...
    };

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment

Re: catalog files simplification

From
Robert Haas
Date:
On Wed, Jun 12, 2019 at 7:52 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> The current catalog files all do this:
>
>     CATALOG(pg_aggregate,2600,AggregateRelationId)
>     {
>         ...
>     } FormData_pg_aggregate;
>
>     typedef FormData_pg_aggregate *Form_pg_aggregate;
>
> The bottom part of this seems redundant.  With the attached patch, we
> can generate that automatically, so this becomes just
>
>     CATALOG(pg_aggregate,2600,AggregateRelationId)
>     {
>         ...
>     };

Maybe the macro definition could be split across several lines instead
of having one really long line?

Are some compilers going to be sad about typedef struct x x; preceding
any declaration or definition of struct x?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: catalog files simplification

From
Tom Lane
Date:
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Jun 12, 2019 at 7:52 AM Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com> wrote:
>> The current catalog files all do this:
>> 
>> CATALOG(pg_aggregate,2600,AggregateRelationId)
>> {
>> ...
>> } FormData_pg_aggregate;
>> 
>> typedef FormData_pg_aggregate *Form_pg_aggregate;
>> 
>> The bottom part of this seems redundant.  With the attached patch, we
>> can generate that automatically, so this becomes just
>> 
>> CATALOG(pg_aggregate,2600,AggregateRelationId)
>> {
>> ...
>> };

> Maybe the macro definition could be split across several lines instead
> of having one really long line?

I think that would complicate Catalog.pm; not clear if it's worth it.

> Are some compilers going to be sad about typedef struct x x; preceding
> any declaration or definition of struct x?

Nope, we have lots of instances of that already, cf "opaque struct"
declarations in various headers.

A bigger objection might be that this would leave us with no obvious-
to-the-untrained-eye declaration point for either the struct name or
the two typedef names.  That might make tools like ctags sad.  Perhaps
it's not really any worse than today, but it bears investigation.

We should also check whether pgindent has any issue with this layout.

            regards, tom lane



Re: catalog files simplification

From
Tom Lane
Date:
I wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> Maybe the macro definition could be split across several lines instead
>> of having one really long line?

> I think that would complicate Catalog.pm; not clear if it's worth it.

Oh, cancel that --- in an uncaffeinated moment, I thought you were asking
about splitting the *call* sites of the CATALOG() macro.  I agree that
the *definition* could be laid out better than it is here.

            regards, tom lane



Re: catalog files simplification

From
Peter Eisentraut
Date:
On 2019-06-12 15:34, Tom Lane wrote:
> A bigger objection might be that this would leave us with no obvious-
> to-the-untrained-eye declaration point for either the struct name or
> the two typedef names.  That might make tools like ctags sad.  Perhaps
> it's not really any worse than today, but it bears investigation.

At least with GNU Global, it finds FormData_pg_foo but not Form_pg_foo.
But you can find the latter using grep.  This patch would hide both of
those even from grep, so maybe it isn't a good idea then.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services