Re: [BUG] Excessive memory usage with update on STORED generated columns.

Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com> writes:
> The actual issue is that ExecComputeStoredGenerated uses
> ri_GeneratedExprsU's NULL-ness to check whether the generated columns'
> expressions have been initialized, whilst for UPDATE ResultRelInfos
> the initialized-indicator is stored in ri_extraUpdatedCols_valid.

Sorry, I missed that you'd already responded.

I don't like using ri_extraUpdatedCols_valid here: it requires callers
to know more than they should about how ExecInitGenerated works, and
it does not fix the comparable performance problem that probably
exists in the INSERT path. I think the right fix is to have three
booleans specifically reflecting the validity of ri_GeneratedExprsU,
ri_GeneratedExprsI, and ri_extraUpdatedCols.

> It also has the benefit of being backportable.

Backportability is definitely a concern, but in this case it
looks like we could get away with squeezing the additional
fields into what is now padding space:

typedef struct ResultRelInfo
{
...
/* For UPDATE, attnums of generated columns to be computed */
Bitmapset *ri_extraUpdatedCols;
/* true if the above has been computed */
bool ri_extraUpdatedCols_valid;

/* Projection to generate new tuple in an INSERT/UPDATE */
ProjectionInfo *ri_projectNew;

There's at least three bytes free after ri_extraUpdatedCols_valid,
so we could cram two more bools in there without an ABI break.
Admittedly, it's not pretty that those bools wouldn't be close
to the fields they describe. But we could improve that in HEAD
with some more-substantial reordering of the contents of
ResultRelInfo; it's not like the current ordering has much rhyme
or reason to it.

regards, tom lane