Re: Symbolic names for the values of typalign and typstorage

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

In response to

Responses

Browse pgsql-hackers by date

  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()