Stephen Frost wrote:
> > > +#define GetModifiedColumns(relinfo, estate) \
> > > + (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->modifiedCols)
> >
> > I assume you are aware that this GetModifiedColumns() macro is a
> > duplicate of another one found elsewhere. However I think this is not
> > such a hot idea; the UPSERT patch series has a preparatory patch that
> > changes that other macro definition, as far as I recall; probably it'd
> > be a good idea to move it elsewhere, to avoid a future divergence.
>
> Yeah, I had meant to do something about that and had looked around but
> didn't find any particularly good place to put it. Any suggestions on
> that?
Hmm, tough call now that I look it up. This macro depends on
ResultRelInfo and EState, both of which are in execnodes.h, and also on
rt_fetch which is in parsetree.h. There is no existing header that
includes parsetree.h (only .c files), so we would have to add one
inclusion on some other header file, or create a new header with just
this definition. None of these sounds real satisfactory (including
parsetree.h in execnodes.h sounds very bad). Maybe just add a comment
on both definitions to note that they are dupes of each other? That
would be more backpatchable that anything else that occurs to me right
away.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services