catalog files simplification

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

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

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

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