Thread: catalog files simplification
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
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