catalog files simplification

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

catalog files simplification

Peter Eisentraut-6
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

0001-Catalog-files-simplification.patch (59K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: catalog files simplification

Robert Haas
On Wed, Jun 12, 2019 at 7:52 AM Peter Eisentraut
<[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: catalog files simplification

Tom Lane-2
Robert Haas <[hidden email]> writes:

> On Wed, Jun 12, 2019 at 7:52 AM Peter Eisentraut
> <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: catalog files simplification

Tom Lane-2
I wrote:
> Robert Haas <[hidden email]> 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