From: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Symbolic names for the values of typalign and typstorage |
Date: | 2020-03-03 01:31:07 |
Message-ID: | 20200303013107.GA26894@alvherre.pgsql |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Justin Pryzby | 2020-03-03 01:31:32 | Re: ALTER tbl rewrite loses CLUSTER ON index |
Previous Message | Peter Geoghegan | 2020-03-03 01:29:11 | Re: Delaying/avoiding BTreeTupleGetNAtts() call within _bt_compare() |