Re: BUG #4417: Foreign keys do not work after altering table/column names

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> Tom Lane wrote:
>> My first thought was some kind of callback function to construct the
>> query text afresh, but that seems pretty baroque. It'd be simpler
>> to just have an option to let plancache.c return a failure indication
>> when its plan is obsolete, whereupon ri_triggers.c discards that plan
>> and builds a fresh one. [ pokes around... ] Hm, the fly in that
>> ointment is that ri_triggers and plancache aren't communicating directly
>> but through SPI. I'm really loath to change the SPI API for this;
>> is there another way?

> ri_triggers.c could register a callback to invalidate entries in its
> hash table with CacheRegisterRelcacheCallback() ?

> Adding a new function, say SPI_is_plan_valid(plan), doesn't seem too bad
> to me either.

It's actually looking kind of nasty. We don't know for sure that the
plan is still valid until RevalidateCachedPlan has succeeded in locking
all the tables involved. (We could look to see if it's already invalid
at some earlier point, but that just leaves us with race conditions.)
And exposing that part of the procedure to a SPI caller would be pretty
horrid --- note the comments in spi.c about not wanting any daylight between
RevalidateCachedPlan and PortalDefineQuery.

The callback solution won't be any fun either. Aside from being a bit
baroque, it would require nontrivial code refactoring in ri_triggers.c
to pull the query text generation code out into separate subroutines.

One possibility is that it looks like all the RI triggers actually
acquire locks on both relations before they do anything interesting.
So maybe there really wouldn't be any race condition if we execute an
is_plan_valid probe at the time of ri_FetchPreparedPlan. It's a bit
shaky but might be better than trying to solve it in a "cleaner"
fashion; certainly anything more invasive than that wouldn't seem like
a good candidate for back-patching.

regards, tom lane