On 2023-Dec-06, Peter Eisentraut wrote:
> One of your (Álvaro's) comments about this earlier was
>
> > Hmm, crazy. I'm not sure I like this, because it seems much too clever.
> > The number of lines that are deleted is alluring, though.
> >
> > Maybe it'd be better to create a separate routine that takes a single
> > ColumnDef and returns the Form_pg_attribute element for it; then use
> > that in both BuildDescForRelation and ATExecAddColumn.
>
> which was also my thought at the beginning. However, this wouldn't quite
> work that way, for several reasons. For instance, BuildDescForRelation()
> also needs to keep track of the has_not_null property across all fields, and
> so if you split that function up, you would have to somehow make that an
> output argument and have the caller keep track of it. Also, the output of
> BuildDescForRelation() in ATExecAddColumn() is passed into
> InsertPgAttributeTuples(), which requires a tuple descriptor anyway, so
> splitting this up into a per-attribute function would then require
> ATExecAddColumn() to re-assemble a tuple descriptor anyway, so this wouldn't
> save anything.
Hmm. Well, if this state of affairs is useful to you, then I withdraw
my objection, because with this patch we're not really adding any new
weirdness, just moving around already-existing weirdness. So let's
press ahead with 0001. (I didn't look at 0002 this time, since
apparently you'd like to process the other patch first and then come
back here.)
If you look closely at InsertPgAttributeTuples and accompanying code, it
all looks a bit archaic. They seem to be treating TupleDesc as a
glorified array of Form_pg_attribute elements in a convenient packaging.
It's probably cleaner to change these APIs so that they deal with a
Form_pg_attribute array, and not TupleDesc anymore. But we can hack on
that some other day.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Pensar que el espectro que vemos es ilusorio no lo despoja de espanto,
sólo le suma el nuevo terror de la locura" (Perelandra, C.S. Lewis)