* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > 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.
Good thought, I'll do that.
Thanks!
Stephen
signature.asc
Description: Digital signature
