Thread: Heads up: 7.3.3 this Wednesday
pgsql-core have agreed to put out a 7.3.3 release on Wednesday (5/21), God willin' an' the creek don't rise. If anyone's got anything you've been planning to fix in the 7.3 branch, now is a real good time to get it done. regards, tom lane
> pgsql-core have agreed to put out a 7.3.3 release on Wednesday (5/21), > God willin' an' the creek don't rise. If anyone's got anything you've > been planning to fix in the 7.3 branch, now is a real good time to get > it done. Only thing is that %rowtype and dropped columns business - but I think you indicated that would be a 7.4 fix...I think it's a bit too complex for me... Chris
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: > Only thing is that %rowtype and dropped columns business - but I think you > indicated that would be a 7.4 fix...I think it's a bit too complex for me... I think only low-risk bug fixes need apply for 7.3 at this point ... the dropped-col thing needs some study ... regards, tom lane
On Fri, 16 May 2003, Tom Lane wrote: > pgsql-core have agreed to put out a 7.3.3 release on Wednesday (5/21), > God willin' an' the creek don't rise. If anyone's got anything you've > been planning to fix in the 7.3 branch, now is a real good time to get > it done. I think Bruce should check his mail box for unapplied patches. I suspect some of our patches are still waiting for attention. Oleg > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster > Regards, Oleg _____________________________________________________________ Oleg Bartunov, sci.researcher, hostmaster of AstroNet, Sternberg Astronomical Institute, Moscow University (Russia) Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/ phone: +007(095)939-16-83, +007(095)939-23-83
Oleg Bartunov <oleg@sai.msu.su> writes: > On Fri, 16 May 2003, Tom Lane wrote: >> pgsql-core have agreed to put out a 7.3.3 release on Wednesday (5/21), > I think Bruce should check his mail box for unapplied patches. > I suspect some of our patches are still waiting for attention. Bruce is going to be mostly out of the loop on this release (he's out of town this weekend, and planning a server upgrade Monday). So if you've got any problems, let me know about 'em. I have just finished digging through the pgsql-patches archives back to February, and found only a few small things that seemed appropriate for back-patching. I do seem to recall seeing some fixes from you and Teodor recently, though. Could you check REL7_3_STABLE CVS tip against what you have, and either resubmit any missing patches or point me to where they're archived? regards, tom lane
I sended it before (24 Apr), but I don't sure that it applied. This patch fixes nt[]-int[] operation Tom Lane wrote: > Oleg Bartunov <oleg@sai.msu.su> writes: > >>On Fri, 16 May 2003, Tom Lane wrote: >> >>>pgsql-core have agreed to put out a 7.3.3 release on Wednesday (5/21), > > >>I think Bruce should check his mail box for unapplied patches. >>I suspect some of our patches are still waiting for attention. > > > Bruce is going to be mostly out of the loop on this release (he's out of > town this weekend, and planning a server upgrade Monday). So if you've > got any problems, let me know about 'em. > > I have just finished digging through the pgsql-patches archives back to > February, and found only a few small things that seemed appropriate for > back-patching. I do seem to recall seeing some fixes from you and > Teodor recently, though. Could you check REL7_3_STABLE CVS tip against > what you have, and either resubmit any missing patches or point me to > where they're archived? > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org -- Teodor Sigaev E-mail: teodor@sigaev.ru
Attachment
Teodor Sigaev <teodor@sigaev.ru> writes: > I sended it before (24 Apr), but I don't sure that it applied. > This patch fixes nt[]-int[] operation Yeah, you're right, it hadn't been applied yet. Done now; thanks! regards, tom lane
Tom Lane wrote: > pgsql-core have agreed to put out a 7.3.3 release on Wednesday (5/21), > God willin' an' the creek don't rise. If anyone's got anything you've > been planning to fix in the 7.3 branch, now is a real good time to get > it done. I've seen no problems with the deferred trigger patch (which addresses a performance issue with deferred triggers) you gave me some time ago. Is this something that's likely to be included in 7.3.3? -- Kevin Brown kevin@sysexperts.com
Kevin Brown <kevin@sysexperts.com> writes: > I've seen no problems with the deferred trigger patch (which addresses > a performance issue with deferred triggers) you gave me some time ago. > Is this something that's likely to be included in 7.3.3? I've been wondering myself about whether to include Jan's patch that reduces foreign-key deadlocks. Neither of these patches have gotten enough testing (that I know of) to make me feel very comfortable about dropping them into 7.3.3 ... but on the other hand, they could be pretty significant fixes. Any votes pro or con? Who else can report successful use of either of these patches? regards, tom lane
> > I've seen no problems with the deferred trigger patch (which > > addresses a performance issue with deferred triggers) you gave me > > some time ago. Is this something that's likely to be included in > > 7.3.3? > > I've been wondering myself about whether to include Jan's patch that > reduces foreign-key deadlocks. Neither of these patches have gotten > enough testing (that I know of) to make me feel very comfortable > about dropping them into 7.3.3 ... but on the other hand, they could > be pretty significant fixes. > > Any votes pro or con? Who else can report successful use of either > of these patches? I've been using the deferred triggers patch in production for about a month without any problems. I'm definitely for that as well as Jan's patch, though I haven't used it. Anything to help out FK's and I'm game. :) If someone needs the clean patch for 7.3 of the deferred triggers, the patch is at the URL below. The one originally posted didn't merge 100% cleanly to my 7.3.2 sources. http://people.FreeBSD.org/~seanc/patch_postgresql-7.3.2::src::backend::utils::adt::ri_triggers.c -sc -- Sean Chittenden
Is is possible to make it a build time switch so many folks can beta test it for us, so to speak? I'd certainly be willing to test it out a bit, but we don't have time to test it before 7.3.3 comes out. That would allow us to basically test the deferred triggers in a relatively stable code base (7.3) on semi-production machines (i.e. the ones running batch files at night and such) long before 7.4 goes beta. Or is it too complex or ugly to put something like that into configure and the code? Just a thought. On Fri, 16 May 2003, Tom Lane wrote: > Kevin Brown <kevin@sysexperts.com> writes: > > I've seen no problems with the deferred trigger patch (which addresses > > a performance issue with deferred triggers) you gave me some time ago. > > Is this something that's likely to be included in 7.3.3? > > I've been wondering myself about whether to include Jan's patch that > reduces foreign-key deadlocks. Neither of these patches have gotten > enough testing (that I know of) to make me feel very comfortable about > dropping them into 7.3.3 ... but on the other hand, they could be pretty > significant fixes. > > Any votes pro or con? Who else can report successful use of either of > these patches? > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faqs/FAQ.html >
"scott.marlowe" <scott.marlowe@ihs.com> writes: > Is is possible to make it a build time switch so many folks can beta test > it for us, so to speak? I doubt it'd get tested enough to notice, if it's not in the default build. I actually think that both of these are pretty good candidates to put into 7.3.3. I'm just trying to adopt an appropriately paranoid stance and ask hard questions about how much they've been tested. Between Kevin and Sean it seems that the deferred-triggers change has gotten enough testing to warrant some trust, but I'm not hearing anything about the FK-deadlock one :-(. BTW, if anyone is looking for that patch, it was at http://archives.postgresql.org/pgsql-hackers/2003-04/msg00260.php regards, tom lane
> > Is is possible to make it a build time switch so many folks can beta test > > it for us, so to speak? > > I doubt it'd get tested enough to notice, if it's not in the default > build. > > I actually think that both of these are pretty good candidates to put > into 7.3.3. I'm just trying to adopt an appropriately paranoid stance > and ask hard questions about how much they've been tested. Between > Kevin and Sean it seems that the deferred-triggers change has gotten > enough testing to warrant some trust, but I'm not hearing anything > about the FK-deadlock one :-(. > > BTW, if anyone is looking for that patch, it was at > http://archives.postgresql.org/pgsql-hackers/2003-04/msg00260.php Are there any test cases to get this bug to fire? I haven't had any problems with this particular bug so I can apply this patch, but I can't promise that my use of the patch will result in anything useful other than, "nothing's broke yet" since I haven't had any real problems with 7.3.2 other than the deferred trigger speed. Off hand, I don't see why this patch would cause any problems if it were applied. -sc -- Sean Chittenden
Sean Chittenden <sean@chittenden.org> writes: >> BTW, if anyone is looking for that patch, it was at >> http://archives.postgresql.org/pgsql-hackers/2003-04/msg00260.php > Are there any test cases to get this bug to fire? I haven't had any > problems with this particular bug so I can apply this patch, but I > can't promise that my use of the patch will result in anything useful > other than, "nothing's broke yet" since I haven't had any real > problems with 7.3.2 other than the deferred trigger speed. "Nothing's broke yet" would be a useful report. I just would like to see some more mileage on the beast before we let it loose on the unsuspecting world. Test cases aren't really the point --- we know what we expect them to do. It's the cases we didn't think of that worry me at times like this. regards, tom lane
On Fri, 16 May 2003, Tom Lane wrote: > "scott.marlowe" <scott.marlowe@ihs.com> writes: > > Is is possible to make it a build time switch so many folks can beta test > > it for us, so to speak? > > I doubt it'd get tested enough to notice, if it's not in the default > build. Well, we could reverse that and make it the default, and if you find a FK problem folks can turn it off. But that probably is sub optimal too.
Did that 'prevent cluster on partial and non-NULL indexes' patch get backported? Chris
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes: > Did that 'prevent cluster on partial and non-NULL indexes' patch get > backported? This one? 2003-03-02 23:37 tgl * src/backend/commands/: cluster.c (REL7_3_STABLE), cluster.c:Prevent clustering on incomplete indexes: partial indexes areverboten,as are non-amindexnulls AMs unless first column isattnotnull. regards, tom lane
"A.Bhuvaneswaran" <bhuvanbk@yahoo.com> writes: > 7.3.2: I applied the above patch and did install and restarted postgresql, > but the 'deadlock detected' error on FK update still exist. The below is > the test case. Someone *advice* me, if it the above mentioned patch is not > intended to address the below case. That is not a foreign-key deadlock; it's a plain old deadlock. It would happen exactly the same way without the foreign key, because the contention is directly for the rows being updated. An example of what the patch fixes: regression=# CREATE TABLE prim_test (id int primary key); NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index 'prim_test_pkey' for table 'prim_test' CREATE TABLE regression=# CREATE TABLE for_test (id int, name text, regression(# ref int references prim_test(id)); NOTICE: CREATE TABLE will create implicit trigger(s) for FOREIGN KEY check(s) CREATE TABLE regression=# INSERT INTO prim_test VALUES ('1'); INSERT 566517 1 regression=# INSERT INTO prim_test VALUES ('2'); INSERT 566518 1 regression=# INSERT INTO for_test VALUES (11, 'foo', 1); INSERT 566520 1 regression=# INSERT INTO for_test VALUES (12, 'fooey', 1); INSERT 566521 1 regression=# INSERT INTO for_test VALUES (21, 'fooey', 2); INSERT 566522 1 regression=# INSERT INTO for_test VALUES (22, 'fooey', 2); INSERT 566523 1 regression=# begin; BEGIN regression=# UPDATE for_test set name ='FOO' where id = 11; UPDATE 1 -- in client 2 do regression=# begin; BEGIN regression=# UPDATE for_test set name = 'BAR' where id = 22; UPDATE 1 regression=# UPDATE for_test set name = 'BAR' where id = 12; UPDATE 1 -- back to client 1, do regression=# UPDATE for_test set name ='FOO' where id = 21; UPDATE 1 This deadlocks in 7.3, but works in CVS tip. regards, tom lane
> regression=# UPDATE for_test set name ='FOO' where id = 21; > UPDATE 1 > > This deadlocks in 7.3, but works in CVS tip. Hmmm...I suspect this will remove a lot of the FK deadlocks I see in my logs all the time...does seem like a bug doesn't it? Chris
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: >> This deadlocks in 7.3, but works in CVS tip. > Hmmm...I suspect this will remove a lot of the FK deadlocks I see in my logs > all the time...does seem like a bug doesn't it? Yeah. I'm just worried that there might be some downside we've not spotted yet. I'd feel better about it if *anyone* had reported successful production use of the patch in the month since it's been available. I thought one or two people had expressed the intention to run the patch when Jan offered it ... where are they? regards, tom lane
> I doubt it'd get tested enough to notice, if it's not in the default > build. > > I actually think that both of these are pretty good candidates to put > into 7.3.3. I'm just trying to adopt an appropriately paranoid stance > and ask hard questions about how much they've been tested. Between > Kevin and Sean it seems that the deferred-triggers change has gotten > enough testing to warrant some trust, but I'm not hearing anything > about the FK-deadlock one :-(. > > BTW, if anyone is looking for that patch, it was at > http://archives.postgresql.org/pgsql-hackers/2003-04/msg00260.php 7.3.2: I applied the above patch and did install and restarted postgresql, but the 'deadlock detected' error on FK update still exist. The below is the test case. Someone *advice* me, if it the above mentioned patch is not intended to address the below case. Test case: test_pg=# CREATE TABLE prim_test (id int primary key); NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index 'prim_test_pkey' for table 'prim_test' CREATE TABLE test_pg=# CREATE TABLE for_test (id int references prim_test(id), name text); NOTICE: CREATE TABLE will create implicit trigger(s) for FOREIGN KEY check(s) CREATE TABLE test_pg=# INSERT INTO prim_test VALUES ('1'); INSERT 4383707 1 test_pg=# INSERT INTO prim_test VALUES ('2'); INSERT 4383708 1 test_pg=# INSERT INTO for_test VALUES (1, 'foo'); INSERT 4383710 1 test_pg=# INSERT INTO for_test VALUES (2, 'bar'); INSERT 4383711 1 t1: test_pg=# BEGIN ; BEGIN test_pg=# UPDATE for_test set name ='FOO' where id = 1; UPDATE 1 test_pg=# UPDATE for_test set name ='Bar' where id = 2; UPDATE 1 t2: test_pg=# BEGIN ; BEGIN test_pg=# UPDATE for_test set name = 'BAR' where id = 2; UPDATE 1 test_pg=# UPDATE for_test set name = 'Foo' where id = 1; ERROR: deadlock detected regards, bhuvaneswaran
Tom Lane wrote: > Yeah. I'm just worried that there might be some downside we've not > spotted yet. I'd feel better about it if *anyone* had reported > successful production use of the patch in the month since it's been > available. I thought one or two people had expressed the intention > to run the patch when Jan offered it ... where are they? I put it in on some DBs we were doing testing against at Liberty; the results unfortunately were inconclusive. We didn't see any noticeable improvement in performance out of it, albeit with limited use, because we weren't planning to use 7.3 in production just yet, and so did the vast majority of the testing against 7.2.4, where the patch doesn't even come close to applying... The result is that I can't suggest either a "yea" or "nay"... -- (reverse (concatenate 'string "gro.gultn@" "enworbbc")) http://www.ntlug.org/~cbbrowne/spiritual.html If con is the opposite of pro, is Congress the opposite of progress?
On Mon, 19 May 2003, A.Bhuvaneswaran wrote: > > I doubt it'd get tested enough to notice, if it's not in the default > > build. > > > > I actually think that both of these are pretty good candidates to put > > into 7.3.3. I'm just trying to adopt an appropriately paranoid stance > > and ask hard questions about how much they've been tested. Between > > Kevin and Sean it seems that the deferred-triggers change has gotten > > enough testing to warrant some trust, but I'm not hearing anything > > about the FK-deadlock one :-(. > > > > BTW, if anyone is looking for that patch, it was at > > http://archives.postgresql.org/pgsql-hackers/2003-04/msg00260.php > > 7.3.2: I applied the above patch and did install and restarted postgresql, > but the 'deadlock detected' error on FK update still exist. The below is > the test case. Someone *advice* me, if it the above mentioned patch is not > intended to address the below case. > > Test case: > > test_pg=# CREATE TABLE prim_test (id int primary key); > NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index > 'prim_test_pkey' > for table 'prim_test' > CREATE TABLE > test_pg=# CREATE TABLE for_test (id int references prim_test(id), name > text); > NOTICE: CREATE TABLE will create implicit trigger(s) for FOREIGN KEY > check(s) > CREATE TABLE > test_pg=# INSERT INTO prim_test VALUES ('1'); > INSERT 4383707 1 > test_pg=# INSERT INTO prim_test VALUES ('2'); > INSERT 4383708 1 > test_pg=# INSERT INTO for_test VALUES (1, 'foo'); > INSERT 4383710 1 > test_pg=# INSERT INTO for_test VALUES (2, 'bar'); > INSERT 4383711 1 > > t1: > test_pg=# BEGIN ; > BEGIN > test_pg=# UPDATE for_test set name ='FOO' where id = 1; > UPDATE 1 > test_pg=# UPDATE for_test set name ='Bar' where id = 2; > UPDATE 1 > > t2: > test_pg=# BEGIN ; > BEGIN > test_pg=# UPDATE for_test set name = 'BAR' where id = 2; > UPDATE 1 > test_pg=# UPDATE for_test set name = 'Foo' where id = 1; > ERROR: deadlock detected Assuming you meant interleaving those statements, that's just a garden variety deadlock (try it again without the foreign key, it still deadlocks).
On Mon, 19 May 2003, Tom Lane wrote: > "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: > >> This deadlocks in 7.3, but works in CVS tip. > > > Hmmm...I suspect this will remove a lot of the FK deadlocks I see in my logs > > all the time...does seem like a bug doesn't it? > > Yeah. I'm just worried that there might be some downside we've not > spotted yet. I'd feel better about it if *anyone* had reported > successful production use of the patch in the month since it's been > available. I thought one or two people had expressed the intention > to run the patch when Jan offered it ... where are they? I'd be glad to test it, but we don't have any issues with fk deadlocks since our load is 99% read, 1% write, and most of the tables with fks on them only have a handful of writers, so any testing I would do would probably just be the "we used it in production and it didn't die" kind of testing.
"scott.marlowe" <scott.marlowe@ihs.com> writes: > I'd be glad to test it, but we don't have any issues with fk deadlocks > since our load is 99% read, 1% write, and most of the tables with fks on > them only have a handful of writers, so any testing I would do would > probably just be the "we used it in production and it didn't die" kind of > testing. That's what I'm looking for mostly: that it does not have any adverse side-effects. regards, tom lane
On Tue, 20 May 2003, Tom Lane wrote: > "scott.marlowe" <scott.marlowe@ihs.com> writes: > > I'd be glad to test it, but we don't have any issues with fk deadlocks > > since our load is 99% read, 1% write, and most of the tables with fks on > > them only have a handful of writers, so any testing I would do would > > probably just be the "we used it in production and it didn't die" kind of > > testing. > > That's what I'm looking for mostly: that it does not have any adverse > side-effects. So where's that patch again? The search function of the mail archives is broken, so I can't seem to find it.
I wrote: > pgsql-core have agreed to put out a 7.3.3 release on Wednesday (5/21), > God willin' an' the creek don't rise. Well, the postgresql.org server move has proven to be much messier than we hoped, so it seems prudent to delay a couple days while the kinks get worked out. New plan is 7.3.3 release on Friday (5/23). regards, tom lane
On Tue, 20 May 2003, scott.marlowe wrote: > On Tue, 20 May 2003, Tom Lane wrote: > > > "scott.marlowe" <scott.marlowe@ihs.com> writes: > > > I'd be glad to test it, but we don't have any issues with fk deadlocks > > > since our load is 99% read, 1% write, and most of the tables with fks on > > > them only have a handful of writers, so any testing I would do would > > > probably just be the "we used it in production and it didn't die" kind of > > > testing. > > > > That's what I'm looking for mostly: that it does not have any adverse > > side-effects. > > So where's that patch again? The search function of the mail archives is > broken, so I can't seem to find it. > did you try http://fts.postgresql.org/ ? It should works > > > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > Regards, Oleg _____________________________________________________________ Oleg Bartunov, sci.researcher, hostmaster of AstroNet, Sternberg Astronomical Institute, Moscow University (Russia) Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/ phone: +007(095)939-16-83, +007(095)939-23-83
scott.marlowe wrote: > On Tue, 20 May 2003, Tom Lane wrote: > > >>"scott.marlowe" <scott.marlowe@ihs.com> writes: >> >>>I'd be glad to test it, but we don't have any issues with fk deadlocks >>>since our load is 99% read, 1% write, and most of the tables with fks on >>>them only have a handful of writers, so any testing I would do would >>>probably just be the "we used it in production and it didn't die" kind of >>>testing. >> >>That's what I'm looking for mostly: that it does not have any adverse >>side-effects. > > > So where's that patch again? The search function of the mail archives is > broken, so I can't seem to find it. > Attached. 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); *** ri_triggers.c.orig Mon Nov 12 01:09:09 2001 --- ri_triggers.c Sun Apr 6 13:36:42 2003 *************** *** 354,366 **** } /* ! * 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(NOTICE, "SPI_connect() failed in RI_FKey_check()"); --- 354,372 ---- } /* ! * 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, NoLock); + return PointerGetDatum(NULL); + } + } + if (SPI_connect() != SPI_OK_CONNECT) elog(NOTICE, "SPI_connect() failed in RI_FKey_check()"); *************** *** 2447,2452 **** --- 2453,2468 ---- if (SPI_finish() != SPI_OK_FINISH) elog(NOTICE, "SPI_finish() failed in RI_FKey_setdefault_del()"); + /* + * 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); /* *************** *** 2722,2727 **** --- 2738,2753 ---- if (SPI_finish() != SPI_OK_FINISH) elog(NOTICE, "SPI_finish() failed in RI_FKey_setdefault_upd()"); + + /* + * 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); *** ri_triggers.c.orig Sat Mar 29 14:33:24 2003 --- ri_triggers.c Sun Apr 6 14:01:27 2003 *************** *** 395,407 **** } /* ! * 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 weird case (this is the only place ! * to see it). */ if (SPI_connect() != SPI_OK_CONNECT) elog(ERROR, "SPI_connect() failed in RI_FKey_check()"); --- 395,413 ---- } /* ! * 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(ERROR, "SPI_connect() failed in RI_FKey_check()"); *************** *** 2397,2402 **** --- 2403,2418 ---- 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); /* *************** *** 2634,2639 **** --- 2650,2665 ---- elog(ERROR, "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);
Thanks! We should just replace the search screen (that doesn't work anyway) on the archives with that one. That fts site rocks! I found it on the ftp server by the way, in the patches directory of all places. I'll put it online on our backup server tonight. On Tue, 20 May 2003, Oleg Bartunov wrote: > On Tue, 20 May 2003, scott.marlowe wrote: > > > On Tue, 20 May 2003, Tom Lane wrote: > > > > > "scott.marlowe" <scott.marlowe@ihs.com> writes: > > > > I'd be glad to test it, but we don't have any issues with fk deadlocks > > > > since our load is 99% read, 1% write, and most of the tables with fks on > > > > them only have a handful of writers, so any testing I would do would > > > > probably just be the "we used it in production and it didn't die" kind of > > > > testing. > > > > > > That's what I'm looking for mostly: that it does not have any adverse > > > side-effects. > > > > So where's that patch again? The search function of the mail archives is > > broken, so I can't seem to find it. > > > > did you try http://fts.postgresql.org/ ? It should works > > > > > > > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 2: you can get off all lists at once with the unregister command > > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > > > > Regards, > Oleg > _____________________________________________________________ > Oleg Bartunov, sci.researcher, hostmaster of AstroNet, > Sternberg Astronomical Institute, Moscow University (Russia) > Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/ > phone: +007(095)939-16-83, +007(095)939-23-83 > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faqs/FAQ.html >
Andrew Sullivan <andrew@libertyrms.info> writes: > Yes, 3 weeks of testing with the patch that Jan provided for 7.2. We > had no failures that I know of. It provided no measurable > performance gain, but it also never deadlocked, and we tested under > conditions where it sometimes did in the past. > Note, however, that we were testing it indirectly; that is, while we > were testing to see if it would break, our application (which does a > number of the referential checks itself, multiple-connection-safety > notwithstanding :( ) tends not to try to violate the foreign keys > anyway. I wouldn't want to claim that we've tested it real heavily. Nonetheless, this does seem to speak to my real concern, which is whether the patch introduces any unexpected side-effects. The code was getting executed, whether or not it detected any FK violations, so we can have some hope that any bizarre problems would have been noticed. I'll go ahead and apply it for 7.3.3. regards, tom lane
On Wed, May 21, 2003 at 01:40:02PM -0400, Tom Lane wrote: > was getting executed, whether or not it detected any FK violations, > so we can have some hope that any bizarre problems would have been > noticed. Certainly, bizarre problems would have been noticed. It's safe to apply, if you ask me. A -- ---- Andrew Sullivan 204-4141 Yonge Street Liberty RMS Toronto, Ontario Canada <andrew@libertyrms.info> M2P 2A8 +1 416 646 3304 x110
Tom Lane wrote: > I actually think that both of these are pretty good candidates to put > into 7.3.3. I'm just trying to adopt an appropriately paranoid stance > and ask hard questions about how much they've been tested. Between > Kevin and Sean it seems that the deferred-triggers change has gotten > enough testing to warrant some trust, but I'm not hearing anything > about the FK-deadlock one :-(. > > BTW, if anyone is looking for that patch, it was at > http://archives.postgresql.org/pgsql-hackers/2003-04/msg00260.php I've been running with this patch ever since I upgraded to 7.3.2 some time back, without any ill effects at all, FWIW... -- Kevin Brown kevin@sysexperts.com
Kevin Brown <kevin@sysexperts.com> writes: > I've been running with this patch ever since I upgraded to 7.3.2 some > time back, without any ill effects at all, FWIW... Andrew Sullivan also reported doing a fair bit of testing without noticing any bad side-effects, so I've gone ahead and applied it for 7.3.3. Appreciate the confirmation though... regards, tom lane
> I doubt it'd get tested enough to notice, if it's not in the default > build. > > I actually think that both of these are pretty good candidates to put > into 7.3.3. I'm just trying to adopt an appropriately paranoid stance > and ask hard questions about how much they've been tested. Between > Kevin and Sean it seems that the deferred-triggers change has gotten > enough testing to warrant some trust, but I'm not hearing anything > about the FK-deadlock one :-(. > > BTW, if anyone is looking for that patch, it was at > http://archives.postgresql.org/pgsql-hackers/2003-04/msg00260.php 7.3.2: I applied the above patch and did install and restarted postgresql, but the 'deadlock detected' error on FK update still exist. The below is the test case. Someone *advice* me, if it the above mentioned patch is not intended to address the below case. Test case: test_pg=# CREATE TABLE prim_test (id int primary key); NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index 'prim_test_pkey' for table 'prim_test' CREATE TABLE test_pg=# CREATE TABLE for_test (id int references prim_test(id), name text); NOTICE: CREATE TABLE will create implicit trigger(s) for FOREIGN KEY check(s) CREATE TABLE test_pg=# INSERT INTO prim_test VALUES ('1'); INSERT 4383707 1 test_pg=# INSERT INTO prim_test VALUES ('2'); INSERT 4383708 1 test_pg=# INSERT INTO for_test VALUES (1, 'foo'); INSERT 4383710 1 test_pg=# INSERT INTO for_test VALUES (2, 'bar'); INSERT 4383711 1 t1: test_pg=# BEGIN ; BEGIN test_pg=# UPDATE for_test set name ='FOO' where id = 1; UPDATE 1 test_pg=# UPDATE for_test set name ='Bar' where id = 2; UPDATE 1 t2: test_pg=# BEGIN ; BEGIN test_pg=# UPDATE for_test set name = 'BAR' where id = 2; UPDATE 1 test_pg=# UPDATE for_test set name = 'Foo' where id = 1; ERROR: deadlock detected regards, bhuvaneswaran