Thread: improve SET CONSTRAINTS
This patch improves the behavior of SET CONSTRAINTS so that it complies with the SQL99 standard, as discussed earlier on -patches. According to SQL99, setting a constraint to IMMEDIATE needs to take effect retroactively: any outstanding deferred modifications need to be checked when the constraint mode is changed, not when the transaction commits. I also took the opportunity to remove a bunch of code in trigger.c that wasn't doing anything useful, AFAICS -- according to SQL99, SET CONSTRAINTS should only apply to the current transaction. So if we're not inside a transaction block currently, SET CONSTRAINTS can return immediately. I also wrote some regression tests for deferred constraints, as well as the new behavior implemented by this patch. I updated the documentation, and all the regression tests pass. Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
Attachment
Neil Conway <nconway@klamath.dyndns.org> writes:
> I also took the opportunity to remove a bunch of code in trigger.c
> that wasn't doing anything useful, AFAICS -- according to SQL99, SET
> CONSTRAINTS should only apply to the current transaction.
I would assume that Jan and Stephan put that code in for a reason.
"Rip out anything I can't find in SQL99" is not the way we do things
around here ...
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Neil Conway <nconway@klamath.dyndns.org> writes: > > I also took the opportunity to remove a bunch of code in trigger.c > > that wasn't doing anything useful, AFAICS -- according to SQL99, SET > > CONSTRAINTS should only apply to the current transaction. > > I would assume that Jan and Stephan put that code in for a reason. In that case, some documentation of its intended purpose would be nice. It doesn't help that the code I cut out was a particularly ugly cut-n-paste job. As an aside, I'm not too fond of adding code with zero present functionality with the vague idea that it may one day be useful. > "Rip out anything I can't find in SQL99" is not the way we do things > around here ... I think that's unfair, and it's certainly not the methodology I'm following. Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
Neil Conway <nconway@klamath.dyndns.org> writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> I would assume that Jan and Stephan put that code in for a reason.
> In that case, some documentation of its intended purpose would be
> nice.
That's a fair complaint. I don't see any sign that that functionality
ever got documented, or even included in the regression tests. Jan?
regards, tom lane
Jan, we need a comment on this. --------------------------------------------------------------------------- Tom Lane wrote: > Neil Conway <nconway@klamath.dyndns.org> writes: > > Tom Lane <tgl@sss.pgh.pa.us> writes: > >> I would assume that Jan and Stephan put that code in for a reason. > > > In that case, some documentation of its intended purpose would be > > nice. > > That's a fair complaint. I don't see any sign that that functionality > ever got documented, or even included in the regression tests. Jan? > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org > -- 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, Pennsylvania 19073
Bruce Momjian wrote: > > Jan, we need a comment on this. I don't recall all the bits and pieces, but looking at the patch it seems to me that it would break SET CONSTRAINTS ALL DEFERRED. The if a constraint is deferrable but not initially deferred, the changes look to me that it'll not become so with this patch. Also, don't we run the trigger queue after each statement anyway? So why does it need to be run by SET CONSTRAINTS explicitly? Jan > > --------------------------------------------------------------------------- > > Tom Lane wrote: > > Neil Conway <nconway@klamath.dyndns.org> writes: > > > Tom Lane <tgl@sss.pgh.pa.us> writes: > > >> I would assume that Jan and Stephan put that code in for a reason. > > > > > In that case, some documentation of its intended purpose would be > > > nice. > > > > That's a fair complaint. I don't see any sign that that functionality > > ever got documented, or even included in the regression tests. Jan? > > > > regards, tom lane > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org > > > > -- > 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, Pennsylvania 19073 > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/users-lounge/docs/faq.html -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #
Jan Wieck <JanWieck@Yahoo.com> writes: > Also, don't we run the trigger queue after each statement anyway? So why > does it need to be run by SET CONSTRAINTS explicitly? I've been pulling my hair out regarding this since Stephen pointed it out earlier on -patches. Although I could have *sworn* I had a test-case in which we did the wrong thing, I can't seem to find one now :-) Sorry for the spam -- AFAICT we did the right thing originally. I've attached a revised patch that just includes the documentation improvements, code cleanup, and regression tests. Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
Attachment
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.
---------------------------------------------------------------------------
Neil Conway wrote:
> Jan Wieck <JanWieck@Yahoo.com> writes:
> > Also, don't we run the trigger queue after each statement anyway? So why
> > does it need to be run by SET CONSTRAINTS explicitly?
>
> I've been pulling my hair out regarding this since Stephen pointed it
> out earlier on -patches. Although I could have *sworn* I had a
> test-case in which we did the wrong thing, I can't seem to find one
> now :-)
>
> Sorry for the spam -- AFAICT we did the right thing originally. I've
> attached a revised patch that just includes the documentation
> improvements, code cleanup, and regression tests.
>
> Cheers,
>
> Neil
>
> --
> Neil Conway <neilconway@rogers.com>
> PGP Key ID: DB3C29FC
[ Attachment, skipping... ]
--
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, Pennsylvania 19073
Patch applied. Thanks. --------------------------------------------------------------------------- Neil Conway wrote: > Jan Wieck <JanWieck@Yahoo.com> writes: > > Also, don't we run the trigger queue after each statement anyway? So why > > does it need to be run by SET CONSTRAINTS explicitly? > > I've been pulling my hair out regarding this since Stephen pointed it > out earlier on -patches. Although I could have *sworn* I had a > test-case in which we did the wrong thing, I can't seem to find one > now :-) > > Sorry for the spam -- AFAICT we did the right thing originally. I've > attached a revised patch that just includes the documentation > improvements, code cleanup, and regression tests. > > Cheers, > > Neil > > -- > Neil Conway <neilconway@rogers.com> > PGP Key ID: DB3C29FC [ Attachment, skipping... ] -- 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, Pennsylvania 19073