Thread: referential constraint bug
To whom it may concern: I found a constraint bug on PostgreSQL lastest version (7.2). Here is how I got the bug. ------------------------------------------------------------------------------------------------ testdb=> testdb=> create table mytest( testdb-> id int primary key, testdb-> masterid int constraint fk_mytest_constr1 REFERENCES mytest(id) ON DELETE CASCADE ON UPDATE NO ACTION DEFERRABLEINITIALLY DEFERRED testdb-> ); NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index 'mytest_pkey' for table 'mytest' NOTICE: CREATE TABLE will create implicit trigger(s) for FOREIGN KEY check(s) CREATE testdb=> begin; BEGIN testdb=> insert into mytest(id,masterid) values(1,null); INSERT 447153 1 testdb=> insert into mytest(id,masterid) values(2,5); INSERT 447154 1 testdb=> insert into mytest(id,masterid) values(3,2); INSERT 447155 1 testdb=> update mytest set masterid=1 where id=2; UPDATE 1 testdb=> end; ERROR: fk_mytest_constr1 referential integrity violation - key referenced from mytest not found in mytest testdb=> ------------------------------------------------------------------------------------------------ Eric PS. 1.There won't be such an error if I do another insert statement which doesn't violate the reference constraint before ending the transaction.(eg.insert into mytest(id,masterid) values(4,3);) 2.It happens while I was using ver7.1.3 and I do an upgrade and run the regression test(everything is fine). 3.It's on a pentium 75(run as 120MHZ) with 40MB RAM, Redhat 6.1
On Wed, 6 Mar 2002, Eric Lu wrote: > To whom it may concern: > > I found a constraint bug on PostgreSQL lastest version (7.2). Yes, it's still seeing some invalid states of the table. I believe the following will fix the case (it was part of a larger patch that was rejected, but I think this part is safe -- if not I'm sure someone will jump in). *** pgsql/src/backend/utils/adt/ri_triggers.c.orig Tue Mar 12 23:33:32 2002 --- pgsql/src/backend/utils/adt/ri_triggers.c Tue Mar 12 23:33:47 2002 *************** *** 208,213 **** --- 208,224 ---- new_row = trigdata->tg_trigtuple; } + /* + * We should not even consider checking the row if it is no longer + * valid since it was either deleted (doesn't matter) or updated + * (in which case it'll be checked with its final values). + */ + if (new_row) { + if (!HeapTupleSatisfiesItself(new_row->t_data)) { + return PointerGetDatum(NULL); + } + } + /* ---------- * SQL3 11.9 <referential constraint definition> * Gereral rules 2) a):
Stephan, let me know if you want this considered for inclusion in CVS. Stephan Szabo wrote: > > On Wed, 6 Mar 2002, Eric Lu wrote: > > > To whom it may concern: > > > > I found a constraint bug on PostgreSQL lastest version (7.2). > > Yes, it's still seeing some invalid states of the table. I believe > the following will fix the case (it was part of a larger patch that > was rejected, but I think this part is safe -- if not I'm sure someone > will jump in). > > *** pgsql/src/backend/utils/adt/ri_triggers.c.orig Tue Mar 12 23:33:32 2002 > --- pgsql/src/backend/utils/adt/ri_triggers.c Tue Mar 12 23:33:47 2002 > *************** > *** 208,213 **** > --- 208,224 ---- > new_row = trigdata->tg_trigtuple; > } > > + /* > + * We should not even consider checking the row if it is no longer > + * valid since it was either deleted (doesn't matter) or updated > + * (in which case it'll be checked with its final values). > + */ > + if (new_row) { > + if (!HeapTupleSatisfiesItself(new_row->t_data)) { > + return PointerGetDatum(NULL); > + } > + } > + > /* ---------- > * SQL3 11.9 <referential constraint definition> > * Gereral rules 2) a): > > > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/users-lounge/docs/faq.html > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
On Wed, 13 Mar 2002, Bruce Momjian wrote: > Stephan, let me know if you want this considered for inclusion in CVS. Probably a good idea in case I can't manage to rewrite the triggers which is involving lots of looking through other code to see how to do stuff. It's fairly safe, but probably not worth putting into 7.2.x if only because we've had the bug since the constraints were added in 7.0 so waiting for the next full version is probably okay (and we can always point users to the patch if they need it).
Your patch has been added to the PostgreSQL unapplied patches list at: http://candle.pha.pa.us/cgi-bin/pgpatches I will try to apply it within the next 48 hours. --------------------------------------------------------------------------- > Stephan, let me know if you want this considered for inclusion in CVS. Probably a good idea in case I can't manage to rewrite the triggers which is involving lots of looking through other code to see how to do stuff. It's fairly safe, but probably not worth putting into 7.2.x if only because we've had the bug since the constraints were added in 7.0 so waiting for the next full version is probably okay (and we can always point users to the patch if they need it). > > On Wed, 6 Mar 2002, Eric Lu wrote: > > > To whom it may concern: > > > > I found a constraint bug on PostgreSQL lastest version (7.2). > > Yes, it's still seeing some invalid states of the table. I believe > the following will fix the case (it was part of a larger patch that > was rejected, but I think this part is safe -- if not I'm sure someone > will jump in). > > *** pgsql/src/backend/utils/adt/ri_triggers.c.orig Tue Mar 12 23:33:32 2002 > --- pgsql/src/backend/utils/adt/ri_triggers.c Tue Mar 12 23:33:47 2002 > *************** > *** 208,213 **** > --- 208,224 ---- > new_row = trigdata->tg_trigtuple; > } > > + /* > + * We should not even consider checking the row if it is no longer > + * valid since it was either deleted (doesn't matter) or updated > + * (in which case it'll be checked with its final values). > + */ > + if (new_row) { > + if (!HeapTupleSatisfiesItself(new_row->t_data)) { > + return PointerGetDatum(NULL); > + } > + } > + > /* ---------- > * SQL3 11.9 <referential constraint definition> > * Gereral rules 2) a): > > > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/users-lounge/docs/faq.html > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Patch applied. Thanks. --------------------------------------------------------------------------- Stephan Szabo wrote: > > On Wed, 6 Mar 2002, Eric Lu wrote: > > > To whom it may concern: > > > > I found a constraint bug on PostgreSQL lastest version (7.2). > > Yes, it's still seeing some invalid states of the table. I believe > the following will fix the case (it was part of a larger patch that > was rejected, but I think this part is safe -- if not I'm sure someone > will jump in). > > *** pgsql/src/backend/utils/adt/ri_triggers.c.orig Tue Mar 12 23:33:32 2002 > --- pgsql/src/backend/utils/adt/ri_triggers.c Tue Mar 12 23:33:47 2002 > *************** > *** 208,213 **** > --- 208,224 ---- > new_row = trigdata->tg_trigtuple; > } > > + /* > + * We should not even consider checking the row if it is no longer > + * valid since it was either deleted (doesn't matter) or updated > + * (in which case it'll be checked with its final values). > + */ > + if (new_row) { > + if (!HeapTupleSatisfiesItself(new_row->t_data)) { > + return PointerGetDatum(NULL); > + } > + } > + > /* ---------- > * SQL3 11.9 <referential constraint definition> > * Gereral rules 2) a): > > > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/users-lounge/docs/faq.html > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026