Thread: RI oddness
Hi, I just got trapped by one of my own features in the referential integrity area. The problem is, that the trigger run on the FK row at UPDATE allways checks and locks the referenced PK, even ifthe FK attributes didn't change. That's because if there'd be an ON DELETE SET DEFAULTS and someone deletes a PKconsisting of all the FK's column defaults, we wouldn't notice and let it pass through. The bad thing on it is now, if I have one XACT that locks the PK row first, then locks the FK row, and I have another XACT that just want's to update another field in the FK row, that second XACT must lock the PK row in the firstplace or this entire thing leads to deadlocks. If one table has alot of FK constraints, this causes not reallywanted lock contention. The clean way to get out of it would be to skip non-FK-change events in the UPDATE trigger and do alot of extra workin the SET DEFAULTS trigger. Actually it'd be to check if we're actually deleting the FK defaults values fromthe PK table, and if so we'd have to check if references exist by doing another NO ACTION kinda test. Any other smart idea? Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com # _________________________________________________________ Do You Yahoo!? Get your free @yahoo.com address at http://mail.yahoo.com
hi, there! On Mon, 23 Apr 2001, Jan Wieck wrote: > I just got trapped by one of my own features in the > referential integrity area. > > The problem is, that the trigger run on the FK row at UPDATE > allways checks and locks the referenced PK, even if the FK > attributes didn't change. That's because if there'd be an ON > DELETE SET DEFAULTS and someone deletes a PK consisting of > all the FK's column defaults, we wouldn't notice and let it > pass through. > > The bad thing on it is now, if I have one XACT that locks the > PK row first, then locks the FK row, and I have another XACT > that just want's to update another field in the FK row, that > second XACT must lock the PK row in the first place or this > entire thing leads to deadlocks. If one table has alot of FK > constraints, this causes not really wanted lock contention. > > The clean way to get out of it would be to skip non-FK-change > events in the UPDATE trigger and do alot of extra work in the > SET DEFAULTS trigger. Actually it'd be to check if we're > actually deleting the FK defaults values from the PK table, > and if so we'd have to check if references exist by doing > another NO ACTION kinda test. > > Any other smart idea? read-write locks? /fjoe
Max Khon wrote: > hi, there! > > On Mon, 23 Apr 2001, Jan Wieck wrote: > > > I just got trapped by one of my own features in the > > referential integrity area. > > > > The problem is, that the trigger run on the FK row at UPDATE > > allways checks and locks the referenced PK, even if the FK > > attributes didn't change. That's because if there'd be an ON > > DELETE SET DEFAULTS and someone deletes a PK consisting of > > all the FK's column defaults, we wouldn't notice and let it > > pass through. > > > > The bad thing on it is now, if I have one XACT that locks the > > PK row first, then locks the FK row, and I have another XACT > > that just want's to update another field in the FK row, that > > second XACT must lock the PK row in the first place or this > > entire thing leads to deadlocks. If one table has alot of FK > > constraints, this causes not really wanted lock contention. > > > > The clean way to get out of it would be to skip non-FK-change > > events in the UPDATE trigger and do alot of extra work in the > > SET DEFAULTS trigger. Actually it'd be to check if we're > > actually deleting the FK defaults values from the PK table, > > and if so we'd have to check if references exist by doing > > another NO ACTION kinda test. > > > > Any other smart idea? > > read-write locks? Just discussed it with Tom Lane while he'd been here in Norfolk and it's even more ugly. We couldn't even pull out the FK's column defaults at this time to check if we are about to delete the corresponding PK because theymight call all kinds of functions with tons of side effects we don't want. Seems the only way to do it cleanly is to have the parser putting the information which TLEs are *OLD* and whichare *NEW* somewhere and pass it all down through the executor (remembering it per tuple in the deferred triggerqueue) down into the triggers. Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com # _________________________________________________________ Do You Yahoo!? Get your free @yahoo.com address at http://mail.yahoo.com
Jan Wieck wrote: > Just discussed it with Tom Lane while he'd been here in > Norfolk and it's even more ugly. We couldn't even pull out > the FK's column defaults at this time to check if we are > about to delete the corresponding PK because they might call > all kinds of functions with tons of side effects we don't > want. > > Seems the only way to do it cleanly is to have the parser > putting the information which TLEs are *OLD* and which are > *NEW* somewhere and pass it all down through the executor > (remembering it per tuple in the deferred trigger queue) down > into the triggers. While we know about the *right* way to fix it, that's a far too big of a change for 7.1.1. But I'd like to fix the likely deadlocks caused by referential integrity constraints. What'd be easy is this: - We already have two entry points for INSERT/UPDATE on FK table, but the one for UPDATE is fortunately unused. - We change analyze.c to install the RI_FKey_check_upd trigger if the constraint has an ON DELETE SET DEFAULT clause. Otherwise it uses RI_FKey_check_ins as it does now. - We change ri_triggers.c so that RI_FKey_check_ins will skip the PK check if the FK attributes did notchange while RI_FKey_check_upd will enforce the check allways. This way it'll automatically gain a performance win for everyone using referential integrity. The bad side effect is, that these changes will require a dump/reload FOR DATABASES, where ON DELETE SET DEFAULT is used. If they don't dump/reload, it'll open the possibility of violating constraints that are defined ON DELETE SET DEFAULT by deleting the PK that consists of the column defaults of an existing FK reference. The DELETE would succeed and the stall references remain. I think the usage of ON DELETE SET DEFAULT is a very rare case out in the field. Thus the dump/reload requirement is limited to a small number of databases (if any). It is easy to detect if a DB's schema contains thisclause by looking up pg_trigger for usage of RI_FKey_setdefault_del. We could provide a small script telling which databases need dump/reload. Comments? Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com # _________________________________________________________ Do You Yahoo!? Get your free @yahoo.com address at http://mail.yahoo.com
Jan Wieck <JanWieck@Yahoo.com> writes: > What'd be easy is this: > - We already have two entry points for INSERT/UPDATE on FK > table, but the one for UPDATE is fortunately unused. > - We change analyze.c to install the RI_FKey_check_upd > trigger if the constraint has an ON DELETE SET DEFAULT > clause. Otherwise it uses RI_FKey_check_ins as it does > now. Unfortunately, such a fix really isn't going to fly as a patch release. Not only does it not work for existing tables, but it won't work for tables created by dump and reload from a prior version (since they won't have the right set of triggers ... another illustration of why the lack of an abstract representation of the RI constraints was a Bad Move). In fact I'm afraid that your proposed change would actively break tables imported from a prior version; wouldn't RI_FKey_check_ins do the wrong thing if applied as an update trigger? > I think the usage of ON DELETE SET DEFAULT is a very rare > case out in the field. Thus the dump/reload requirement is > limited to a small number of databases (if any). But dump/reload won't fix the tables' triggers. Given that ON DELETE SET DEFAULT isn't used much, I think we should not waste time creating an incomplete hack solution for 7.1.*, but just write it off as a known bug and move forward with a real solution for 7.2. regards, tom lane
Tom Lane wrote: > Jan Wieck <JanWieck@Yahoo.com> writes: > > What'd be easy is this: > > > - We already have two entry points for INSERT/UPDATE on FK > > table, but the one for UPDATE is fortunately unused. > > > - We change analyze.c to install the RI_FKey_check_upd > > trigger if the constraint has an ON DELETE SET DEFAULT > > clause. Otherwise it uses RI_FKey_check_ins as it does > > now. > > Unfortunately, such a fix really isn't going to fly as a patch release. > Not only does it not work for existing tables, but it won't work for > tables created by dump and reload from a prior version (since they > won't have the right set of triggers ... another illustration of why > the lack of an abstract representation of the RI constraints was a > Bad Move). In fact I'm afraid that your proposed change would actively > break tables imported from a prior version; wouldn't RI_FKey_check_ins > do the wrong thing if applied as an update trigger? > > > I think the usage of ON DELETE SET DEFAULT is a very rare > > case out in the field. Thus the dump/reload requirement is > > limited to a small number of databases (if any). > > But dump/reload won't fix the tables' triggers. Ech - you're right. It wouldn't fix 'em. > > Given that ON DELETE SET DEFAULT isn't used much, I think we should > not waste time creating an incomplete hack solution for 7.1.*, but > just write it off as a known bug and move forward with a real solution > for 7.2. It's not the rarely used ON DELETE SET DEFAULT case that's currently broken. It's ALL the other cases that can easily cause you to end up in deadlocks if you just update another field in a table having foreign keys and youdon't lock all referenced rows properly first. Given the table: CREATE TABLE sample ( a integer REFERENCES t1, b integer REFERENCES t2, c integer REFERENCES t3, d integer REFERENCES t4, data text ); you'd have to SELECT ... FOR UPDATE tables t1, t2, t3 and t4 (while NOT having a lock on "sample") before you can safely update "data". Otherwise, another transaction could lock one of those and try to lock your "sample" row and you have a deadlock. We could provide another script fixing it. It is run after the restore of a dump taken from a pre-7.1.1 database fixing the tgfoid for those triggers that use RI_FKey_check_ins where a matching RI_FKey_setdefault_delrow exist with same arguments and constraint name. Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com # _________________________________________________________ Do You Yahoo!? Get your free @yahoo.com address at http://mail.yahoo.com