Re: Backpatch FK changes to 7.3 and 7.2? - Mailing list pgsql-hackers

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@Yahoo.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@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073