Re: Backpatch FK changes to 7.3 and 7.2?

Seems like a small reasonable patch to me, and several folks want it.

---------------------------------------------------------------------------

Jan Wieck wrote:
> Stephan Szabo wrote:
> >
> > On Tue, 8 Apr 2003, Tatsuo Ishii wrote:
> >
> > > > > The changes I committed to address most of the FK deadlock problems
> > > > > reported can easily be applied to the 7.3 and 7.2 source trees as well.
> > > > >
> > > > > Except for a slight change in the text of the error message that gets
> > > > > thrown "if one tries to delete a referenced PK for which a FK with ON
> > > > > DELETE SET DEFAULT exists" (it's a rare case, believe me), this patch
> > > > > would qualify for backpatching. The unnecessary FOR UPDATE lock of
> > > > > referenced rows could be counted as a bug.
> > > > >
> > > > > Opinions?
> > > >
> > > > Since I seem to suffer from these horrible deadlock problems all the
> > > > time, I'd like it to be backported to 7.3...
> > >
> > > Me too!
> >
> > As a note, this'll solve some of the deadlocks on fk update (generally the
> > key values aren't touched) but not insert related ones (two rows inserted
> > to the same primary key causing one to wait and possible deadlocks)
> >
> > In any case, why don't we get a patch against 7.3, and make an
> > announcement and let people who are interested use it and test it. With
> > in-field testing it'd probably be safe enough. :)
>
> Here it is.
>
>
> Jan
>
> --
> #======================================================================#
> # It's easier to get forgiveness for being wrong than for being right. #
> # Let's break this rule - forgive me. #
> #================================================== JanWieck(at)Yahoo(dot)com #

> *** ri_triggers.c.orig Fri Apr 4 10:41:45 2003
> --- ri_triggers.c Sun Apr 6 12:36:54 2003
> ***************
> *** 391,403 ****
> }
>
> /*
> ! * Note: We cannot avoid the check on UPDATE, even if old and new key
> ! * are the same. Otherwise, someone could DELETE the PK that consists
> ! * of the DEFAULT values, and if there are any references, a ON DELETE
> ! * SET DEFAULT action would update the references to exactly these
> ! * values but we wouldn't see that weired case (this is the only place
> ! * to see it).
> */
> if (SPI_connect() != SPI_OK_CONNECT)
> elog(WARNING, "SPI_connect() failed in RI_FKey_check()");
>
> --- 391,409 ----
> }
>
> /*
> ! * No need to check anything if old and new references are the
> ! * same on UPDATE.
> */
> + if (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
> + {
> + if (ri_KeysEqual(fk_rel, old_row, new_row, &qkey,
> + RI_KEYPAIR_FK_IDX))
> + {
> + heap_close(pk_rel, RowShareLock);
> + return PointerGetDatum(NULL);
> + }
> + }
> +
> if (SPI_connect() != SPI_OK_CONNECT)
> elog(WARNING, "SPI_connect() failed in RI_FKey_check()");
>
> ***************
> *** 2787,2792 ****
> --- 2793,2808 ----
>
> heap_close(fk_rel, RowExclusiveLock);
>
> + /*
> + * In the case we delete the row who's key is equal to the
> + * default values AND a referencing row in the foreign key
> + * table exists, we would just have updated it to the same
> + * values. We need to do another lookup now and in case a
> + * reference exists, abort the operation. That is already
> + * implemented in the NO ACTION trigger.
> + */
> + RI_FKey_noaction_del(fcinfo);
> +
> return PointerGetDatum(NULL);
>
> /*
> ***************
> *** 3077,3082 ****
> --- 3093,3108 ----
> elog(WARNING, "SPI_finish() failed in RI_FKey_setdefault_upd()");
>
> heap_close(fk_rel, RowExclusiveLock);
> +
> + /*
> + * In the case we updated the row who's key was equal to the
> + * default values AND a referencing row in the foreign key
> + * table exists, we would just have updated it to the same
> + * values. We need to do another lookup now and in case a
> + * reference exists, abort the operation. That is already
> + * implemented in the NO ACTION trigger.
> + */
> + RI_FKey_noaction_upd(fcinfo);
>
> return PointerGetDatum(NULL);
>

>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073