On 2020-Mar-02, Tom Lane wrote:
> While looking at Tomas' ALTER TYPE patch, I got annoyed by the fact
> that all of the backend writes constants of type alignment and type
> storage values as literal characters, such as 'i' and 'x'. This is
> not our style for most other "poor man's enum" catalog columns, and
> it makes it really hard to grep for relevant code. Hence, attached
> is a proposed patch to invent #define names for those values.
Makes sense.
> As is our custom for other similar catalog columns, I only used the
> macros in C code. There are some references in SQL code too,
> particularly in the regression tests, but the difficulty of replacing
> symbolic references in SQL code seems more than it's worth to fix.
Agreed.
> One thing that I'm not totally happy about, as this stands, is that
> we have to #include "catalog/pg_type.h" in various places we did
> not need to before (although only a fraction of the files I touched
> need that). Part of the issue is that I used the TYPALIGN_XXX
> macros in tupmacs.h, but did not #include pg_type.h there because
> I was concerned about macro inclusion bloat. Plausible alternatives
> to the way I did it here include
>
> * just bite the bullet and #include pg_type.h in tupmacs.h;
I like this one the most -- better than the alternative in the patch --
because it's the most honest IMO, except that there seems to be
altogether too much cruft in pg_type.h that should be elsewhere
(particularly nodes/nodes.h, which includes a large number of other
headers).
If we think that pg_type.h is the header to handle access to the pg_type
catalog, then I would think that the function declarations at the bottom
should be in some "internal" header file; then we can get rid of most
the #includes in pg_type.h.
> Thoughts? Anybody want to say that this is more code churn
> than it's worth?
It seems worthy cleanup to me.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
