Thread: Nested xacts: looking for testers and review
Hackers, Ok, I've finally coded solutions to most problems regarding nested transactions. This means: - reversing for the lock manager, catcache, relcache, buffer manager, asynchronous notifies, storage manager. - transaction block state support, including appropiate XLog recording - pg_subtrans subsystem (including changing state from SUBTRANS COMMITTED to COMMITTED when appropiate). Also pg_clog XLogrecovery was handed to SLRU so pg_subtrans and pg_clog share a rmgr identity. - visibility rules. I'm missing one item: deferred triggers. The problem with this is that the deftrig queue is not implemented using normal Lists, so there's no efficient way to reassign to the parent when the subtransaction commits. Also I'm not sure what should happen to the "immediate" pointer --- a subtransaction should have it's own private copy, or it should inherit the parent's? Please whoever implemented this speak up (Stephan Szabo?), as I'm not sure of the semantics. I have tested it and it passes all regression tests (including ones I added), plus some more tests I threw at it mainly for concurrency. Everything behaves as expected. At this time I'd like to have it reviewed by the critic eye of the committers, and tested by whoever would be using it. I'm open for comments and suggestions and general input. Thank you. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) La web junta la gente porque no importa que clase de mutante sexual seas, tienes millones de posibles parejas. Pon "buscar gente que tengan sexo con ciervos incendiánse", y el computador dirá "especifique el tipo de ciervo" (Jason Alexander)
On Wed, 26 May 2004, Alvaro Herrera wrote: > I'm missing one item: deferred triggers. The problem with this is that > the deftrig queue is not implemented using normal Lists, so there's no > efficient way to reassign to the parent when the subtransaction commits. > Also I'm not sure what should happen to the "immediate" pointer --- a > subtransaction should have it's own private copy, or it should inherit > the parent's? Please whoever implemented this speak up (Stephan > Szabo?), as I'm not sure of the semantics. [I haven't had a chance to look at the patch, because I'm at work, so you may already have throught about/coded for some of the things below, but...] The immediate pointer basically points at the end of the queue from the last scanning of the trigger queue (since any "immediate" triggers from before that would have been run at that time there's no reason to scan from the beginning). As a related side question I just thought of (and wish I had earlier), what's the correct behavior of SET CONSTRAINTS when setting a constraint to immediate mode in the subtransaction case? In the single transaction case, we change whether we consider the constraint deferred (or set the "all"), reset the immediate pointer to the beginning of the queue, and allow the end of statement scan to do what should hopefully be the right thing. If one sets a constraint to immediate in a subtransaction, does/should it cause the immediate check of pending events from its parent? And does that revert when the transaction ends? Before thinking about that, I figured that each subtransaction would have a private trigger queue and then that the immediate pointer would be private in each queue (starting as NULL when the subtransaction started) and that if items were copied/reassigned at subtransaction end, the parent transaction's immediate would move to just after that point after the copy. Now, I'm not entirely sure.
On Wed, May 26, 2004 at 04:35:52PM -0700, Stephan Szabo wrote: > On Wed, 26 May 2004, Alvaro Herrera wrote: > > > I'm missing one item: deferred triggers. The problem with this is that > > the deftrig queue is not implemented using normal Lists, so there's no > > efficient way to reassign to the parent when the subtransaction commits. > > Also I'm not sure what should happen to the "immediate" pointer --- a > > subtransaction should have it's own private copy, or it should inherit > > the parent's? Please whoever implemented this speak up (Stephan > > Szabo?), as I'm not sure of the semantics. > > The immediate pointer basically points at the end of the queue from the > last scanning of the trigger queue (since any "immediate" triggers from > before that would have been run at that time there's no reason to scan > from the beginning). Hmm. You assume correctly that a subtransaction has (or will have) a private queue. But we do not consider a subtransaction to be somewhat a separate entity -- the principle is that the transaction has to feel just like the BEGIN wasn't there. So BEGIN;UPDATE foo ...UPDATE bar ... COMMIT has to be exactly the same as BEGIN;BEGIN; UPDATE foo ...COMMIT;BEGIN; UPDATE bar ...COMMIT; COMMIT; Now, with that in mind: is it correct that the "immediate" pointer points to the beginning of the subtransaction's private deferred trigger queue, at subtransaction's start? Now, at subtransaction end, lets suppose I concatenate the list the original transaction had with the subtransaction's private list. What should the immediate pointer be? When is the immediate pointer advanced? I know it's "during scanning of the list", but when is this? At the end of each query? > If one sets a constraint to immediate in a subtransaction, does/should > it cause the immediate check of pending events from its parent? And > does that revert when the transaction ends? Yes, I think it should fire all events, including the parent's. Good point; it means there has to be a way of getting the whole list, from the topmost transaction, in order :-( I'm not sure what you mean by reverting though. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) La web junta la gente porque no importa que clase de mutante sexual seas, tienes millones de posibles parejas. Pon "buscar gente que tengan sexo con ciervos incendiánse", y el computador dirá "especifique el tipo de ciervo" (Jason Alexander)
On Thu, 27 May 2004, Alvaro Herrera wrote: > On Wed, May 26, 2004 at 04:35:52PM -0700, Stephan Szabo wrote: > > > On Wed, 26 May 2004, Alvaro Herrera wrote: > > > > > I'm missing one item: deferred triggers. The problem with this is that > > > the deftrig queue is not implemented using normal Lists, so there's no > > > efficient way to reassign to the parent when the subtransaction commits. > > > Also I'm not sure what should happen to the "immediate" pointer --- a > > > subtransaction should have it's own private copy, or it should inherit > > > the parent's? Please whoever implemented this speak up (Stephan > > > Szabo?), as I'm not sure of the semantics. > > > > The immediate pointer basically points at the end of the queue from the > > last scanning of the trigger queue (since any "immediate" triggers from > > before that would have been run at that time there's no reason to scan > > from the beginning). > > Hmm. You assume correctly that a subtransaction has (or will have) a > private queue. But we do not consider a subtransaction to be somewhat a > separate entity -- the principle is that the transaction has to feel > just like the BEGIN wasn't there. So > > BEGIN; > UPDATE foo ... > UPDATE bar ... > COMMIT > > has to be exactly the same as > > BEGIN; > BEGIN; > UPDATE foo ... > COMMIT; > BEGIN; > UPDATE bar ... > COMMIT; > COMMIT; > > Now, with that in mind: is it correct that the "immediate" pointer > points to the beginning of the subtransaction's private deferred trigger > queue, at subtransaction's start? AFAIR you can set it to NULL because that means scan the entire list. > Now, at subtransaction end, lets suppose I concatenate the list the > original transaction had with the subtransaction's private list. What > should the immediate pointer be? I'd say pointing at the last item that was added to the queue. > When is the immediate pointer advanced? I know it's "during scanning of > the list", but when is this? At the end of each query? At the point after triggers fire after a statement. It's the actual scanning of the trigger queue to see what to run that changes it unless I'm misremembering. > > If one sets a constraint to immediate in a subtransaction, does/should > > it cause the immediate check of pending events from its parent? And > > does that revert when the transaction ends? > > Yes, I think it should fire all events, including the parent's. Good > point; it means there has to be a way of getting the whole list, from > the topmost transaction, in order :-( Yeah... Right now we don't need to do something special because resetting the immediate pointer basically does what we want (re-scan the entire set looking for non-run things that are now immediate). > I'm not sure what you mean by reverting though. The state about whether a trigger is actually deferred or immediate. I believe it basically works as: begin;set constraints all immediate;-- here any deferrable constraints are treated as immediate end; begin;-- here any deferrable constraints are in their default state end; So, if you have begin;-- 1begin; set constraints all immediate;end;-- 2 end; Do 1 and 2 see the same constraint checking mode or is 2 at immediate?
I have tested the patches with May 28 16:20 JST snapshot. Here is my first impression: 1) errors, rules regression tests are failed (I'm not sure this is due to your patches) 2) certain behavior was different from what I expected (please correct me if my expectation is wrong). test=# begin; BEGIN test=# insert into t1 values(1); INSERT 17216 1 test=# begin; BEGIN test=# aaa; ERROR: syntax error at or near "aaa" at character 1 ERROR: syntax error at or near "aaa" at character 1 LINE 1: aaa; ^ test=# end; COMMIT test=# select * from t1; <-- I thought this should work since subtransaction was closed ERROR: current transaction is aborted, commands ignored until end of transaction block ERROR: current transaction is aborted, commands ignored until end of transaction block test=# end; COMMIT test=# select * from t1;i --- (0 rows) 3) no docs found in the patches. > Hackers, > > Ok, I've finally coded solutions to most problems regarding nested > transactions. This means: > > - reversing for the lock manager, catcache, relcache, buffer manager, > asynchronous notifies, storage manager. > > - transaction block state support, including appropiate XLog recording > > - pg_subtrans subsystem (including changing state from SUBTRANS > COMMITTED to COMMITTED when appropiate). Also pg_clog XLog recovery > was handed to SLRU so pg_subtrans and pg_clog share a rmgr identity. > > - visibility rules. > > I'm missing one item: deferred triggers. The problem with this is that > the deftrig queue is not implemented using normal Lists, so there's no > efficient way to reassign to the parent when the subtransaction commits. > Also I'm not sure what should happen to the "immediate" pointer --- a > subtransaction should have it's own private copy, or it should inherit > the parent's? Please whoever implemented this speak up (Stephan > Szabo?), as I'm not sure of the semantics. > > > I have tested it and it passes all regression tests (including ones I > added), plus some more tests I threw at it mainly for concurrency. > Everything behaves as expected. At this time I'd like to have it > reviewed by the critic eye of the committers, and tested by whoever > would be using it. > > I'm open for comments and suggestions and general input. Thank you. > > -- > Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) > La web junta la gente porque no importa que clase de mutante sexual seas, > tienes millones de posibles parejas. Pon "buscar gente que tengan sexo con > ciervos incendiánse", y el computador dirá "especifique el tipo de ciervo" > (Jason Alexander) > > > ---------------------------(end of broadcast)--------------------------- > TIP 7: don't forget to increase your free space map settings >
I think before this can be tested fully by a wide audience there needs to
be some basic documentation. Or is there?
What is the new syntax?
Can we see some basic examples that you have used in your testing?
Regards
Bob
|---------+---------------------------------->
| | Tatsuo Ishii |
| | <t-ishii@sra.co.jp> |
| | Sent by: |
| | pgsql-hackers-owner@pos|
| | tgresql.org |
| | |
| | |
| | 05/28/2004 02:51 AM |
| | |
|---------+---------------------------------->
>------------------------------------------------------------------------------------------------------------------------------|
|
| | To: alvherre@dcc.uchile.cl
| | cc: pgsql-hackers@postgresql.org
| | Subject: Re: [HACKERS] Nested xacts: looking for testers and review
|
>------------------------------------------------------------------------------------------------------------------------------|
I have tested the patches with May 28 16:20 JST snapshot. Here is my
first impression:
1) errors, rules regression tests are failed (I'm not sure this is due to your patches)
2) certain behavior was different from what I expected (please correct me if my expectation is wrong).
test=# begin;
BEGIN
test=# insert into t1 values(1);
INSERT 17216 1
test=# begin;
BEGIN
test=# aaa;
ERROR: syntax error at or near "aaa" at character 1
ERROR: syntax error at or near "aaa" at character 1
LINE 1: aaa; ^
test=# end;
COMMIT
test=# select * from t1; <-- I thought this should work since
subtransaction was closed
ERROR: current transaction is aborted, commands ignored until end of
transaction block
ERROR: current transaction is aborted, commands ignored until end of
transaction block
test=# end;
COMMIT
test=# select * from t1;i
---
(0 rows)
3) no docs found in the patches.
> Hackers,
>
> Ok, I've finally coded solutions to most problems regarding nested
> transactions. This means:
>
> - reversing for the lock manager, catcache, relcache, buffer manager,
> asynchronous notifies, storage manager.
>
> - transaction block state support, including appropiate XLog recording
>
> - pg_subtrans subsystem (including changing state from SUBTRANS
> COMMITTED to COMMITTED when appropiate). Also pg_clog XLog recovery
> was handed to SLRU so pg_subtrans and pg_clog share a rmgr identity.
>
> - visibility rules.
>
> I'm missing one item: deferred triggers. The problem with this is that
> the deftrig queue is not implemented using normal Lists, so there's no
> efficient way to reassign to the parent when the subtransaction commits.
> Also I'm not sure what should happen to the "immediate" pointer --- a
> subtransaction should have it's own private copy, or it should inherit
> the parent's? Please whoever implemented this speak up (Stephan
> Szabo?), as I'm not sure of the semantics.
>
>
> I have tested it and it passes all regression tests (including ones I
> added), plus some more tests I threw at it mainly for concurrency.
> Everything behaves as expected. At this time I'd like to have it
> reviewed by the critic eye of the committers, and tested by whoever
> would be using it.
>
> I'm open for comments and suggestions and general input. Thank you.
>
> --
> Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
> La web junta la gente porque no importa que clase de mutante sexual seas,
> tienes millones de posibles parejas. Pon "buscar gente que tengan sexo
con
> ciervos incendiánse", y el computador dirá "especifique el tipo de
ciervo"
> (Jason Alexander)
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings
>
---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings
*************************************************************************
PRIVILEGED AND CONFIDENTIAL: This communication, including attachments, is for the exclusive use of addressee and may
containproprietary, confidential and/or privileged information. If you are not the intended recipient, any use,
copying,disclosure, dissemination or distribution is strictly prohibited. If you are not the intended recipient,
pleasenotify the sender immediately by return e-mail, delete this communication and destroy all copies.
*************************************************************************
This reply was meant to be directed to Alvaro.
|---------+---------------------------------->
| | Bob.Henkel@hartfordlife|
| | .com |
| | Sent by: |
| | pgsql-hackers-owner@pos|
| | tgresql.org |
| | |
| | |
| | 05/28/2004 07:45 AM |
| | |
|---------+---------------------------------->
>------------------------------------------------------------------------------------------------------------------------------|
|
| | To: Tatsuo Ishii <t-ishii@sra.co.jp>
| | cc: alvherre@dcc.uchile.cl, pgsql-hackers@postgresql.org,
pgsql-hackers-owner@postgresql.org | | Subject: Re: [HACKERS] Nested xacts: looking for
testersand review |
>------------------------------------------------------------------------------------------------------------------------------|
I think before this can be tested fully by a wide audience there needs to
be some basic documentation. Or is there?
What is the new syntax?
Can we see some basic examples that you have used in your testing?
Regards
Bob
|---------+---------------------------------->
| | Tatsuo Ishii |
| | <t-ishii@sra.co.jp> |
| | Sent by: |
| | pgsql-hackers-owner@pos|
| | tgresql.org |
| | |
| | |
| | 05/28/2004 02:51 AM |
| | |
|---------+----------------------------------> >
------------------------------------------------------------------------------------------------------------------------------|
|
| | To: alvherre@dcc.uchile.cl
| | cc: pgsql-hackers@postgresql.org
| | Subject: Re: [HACKERS] Nested xacts: looking for testers and
review | >
------------------------------------------------------------------------------------------------------------------------------|
I have tested the patches with May 28 16:20 JST snapshot. Here is my
first impression:
1) errors, rules regression tests are failed (I'm not sure this is due to your patches)
2) certain behavior was different from what I expected (please correct me if my expectation is wrong).
test=# begin;
BEGIN
test=# insert into t1 values(1);
INSERT 17216 1
test=# begin;
BEGIN
test=# aaa;
ERROR: syntax error at or near "aaa" at character 1
ERROR: syntax error at or near "aaa" at character 1
LINE 1: aaa; ^
test=# end;
COMMIT
test=# select * from t1; <-- I thought this should work since
subtransaction was closed
ERROR: current transaction is aborted, commands ignored until end of
transaction block
ERROR: current transaction is aborted, commands ignored until end of
transaction block
test=# end;
COMMIT
test=# select * from t1;i
---
(0 rows)
3) no docs found in the patches.
> Hackers,
>
> Ok, I've finally coded solutions to most problems regarding nested
> transactions. This means:
>
> - reversing for the lock manager, catcache, relcache, buffer manager,
> asynchronous notifies, storage manager.
>
> - transaction block state support, including appropiate XLog recording
>
> - pg_subtrans subsystem (including changing state from SUBTRANS
> COMMITTED to COMMITTED when appropiate). Also pg_clog XLog recovery
> was handed to SLRU so pg_subtrans and pg_clog share a rmgr identity.
>
> - visibility rules.
>
> I'm missing one item: deferred triggers. The problem with this is that
> the deftrig queue is not implemented using normal Lists, so there's no
> efficient way to reassign to the parent when the subtransaction commits.
> Also I'm not sure what should happen to the "immediate" pointer --- a
> subtransaction should have it's own private copy, or it should inherit
> the parent's? Please whoever implemented this speak up (Stephan
> Szabo?), as I'm not sure of the semantics.
>
>
> I have tested it and it passes all regression tests (including ones I
> added), plus some more tests I threw at it mainly for concurrency.
> Everything behaves as expected. At this time I'd like to have it
> reviewed by the critic eye of the committers, and tested by whoever
> would be using it.
>
> I'm open for comments and suggestions and general input. Thank you.
>
> --
> Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
> La web junta la gente porque no importa que clase de mutante sexual seas,
> tienes millones de posibles parejas. Pon "buscar gente que tengan sexo
con
> ciervos incendiánse", y el computador dirá "especifique el tipo de
ciervo"
> (Jason Alexander)
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings
>
---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings
*************************************************************************
PRIVILEGED AND CONFIDENTIAL: This communication, including attachments, is
for the exclusive use of addressee and may contain proprietary,
confidential and/or privileged information. If you are not the intended
recipient, any use, copying, disclosure, dissemination or distribution is
strictly prohibited. If you are not the intended recipient, please notify
the sender immediately by return e-mail, delete this communication and
destroy all copies.
*************************************************************************
---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to
majordomo@postgresql.orgso that your message can get through to the mailing list cleanly
*************************************************************************
PRIVILEGED AND CONFIDENTIAL: This communication, including attachments, is for the exclusive use of addressee and may
containproprietary, confidential and/or privileged information. If you are not the intended recipient, any use,
copying,disclosure, dissemination or distribution is strictly prohibited. If you are not the intended recipient,
pleasenotify the sender immediately by return e-mail, delete this communication and destroy all copies.
*************************************************************************
On Fri, May 28, 2004 at 04:51:07PM +0900, Tatsuo Ishii wrote: > 2) certain behavior was different from what I expected (please correct me > if my expectation is wrong). Yes, the expected behavior is different: if an aborted subtransaction is closed with a COMMIT or END command, the parent transaction is aborted too. This is to inhibit an application which blindly expects the subtransaction to succeed to reach an invalid state. If you want to return to non-aborted state, end the subtransaction with ROLLBACK instead. But Bob Henkel and you are right: there needo to be documentation. Initially I figured I could do that later because there is no new syntax, but it is obviously needed to explain all sorts of assumptions and behavior like this. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Limítate a mirar... y algun día veras"
> On Fri, May 28, 2004 at 04:51:07PM +0900, Tatsuo Ishii wrote: > > > 2) certain behavior was different from what I expected (please correct me > > if my expectation is wrong). > > Yes, the expected behavior is different: if an aborted subtransaction is > closed with a COMMIT or END command, the parent transaction is aborted > too. This is to inhibit an application which blindly expects the > subtransaction to succeed to reach an invalid state. If you want to > return to non-aborted state, end the subtransaction with ROLLBACK > instead. Oh, I got it. Thanks. > But Bob Henkel and you are right: there needo to be documentation. > Initially I figured I could do that later because there is no new > syntax, but it is obviously needed to explain all sorts of assumptions > and behavior like this. -- Tatsuo Ishii
Can I ask you one more question? Is there any limit for nesting leveles of subtransactions? -- Tatsuo Ishii > On Fri, May 28, 2004 at 04:51:07PM +0900, Tatsuo Ishii wrote: > > > 2) certain behavior was different from what I expected (please correct me > > if my expectation is wrong). > > Yes, the expected behavior is different: if an aborted subtransaction is > closed with a COMMIT or END command, the parent transaction is aborted > too. This is to inhibit an application which blindly expects the > subtransaction to succeed to reach an invalid state. If you want to > return to non-aborted state, end the subtransaction with ROLLBACK > instead. > > But Bob Henkel and you are right: there needo to be documentation. > Initially I figured I could do that later because there is no new > syntax, but it is obviously needed to explain all sorts of assumptions > and behavior like this. > > -- > Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) > "Limítate a mirar... y algun día veras" > > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster >
On Sat, May 29, 2004 at 12:27:39AM +0900, Tatsuo Ishii wrote: > Can I ask you one more question? > > Is there any limit for nesting leveles of subtransactions? In theory 2^16 I think, but I haven't tested it. It tried to 30 or so only. Not sure if it's practical. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Escucha y olvidarás; ve y recordarás; haz y entenderás" (Confucio)
On Fri, May 28, 2004 at 04:05:40PM -0400, Bruce Momjian wrote: Hm, you are right that there needs to be a more automatic way of doing this. > One interesting idea would be for COMMIT to affect the outer > transaction, and END not affect the outer transaction. Of course that > kills the logic that COMMIT and END are the same, but it is an > interesting idea, and doesn't affect backward compatibility because > END/COMMIT behave the same in non-nested transactions. How about "COMMIT SUB" and "END SUB"? I don't feel it's good to give different meaning to COMMIT versus END, but this is only a gut kind of thing and I could be convinced otherwise. It is even easier to differentiate COMMIT/END than adding a parameter to them. I mean, COMMIT SUB would not affect the state of the outer transaction, while COMMIT would. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "In fact, the basic problem with Perl 5's subroutines is that they're not crufty enough, so the cruft leaks out into user-defined code instead, by the Conservation of Cruft Principle." (Larry Wall, Apocalypse 6)
On Fri, May 28, 2004 at 01:43:16PM -0400, Bruce Momjian wrote: > In this case, I want to try all of the inserts, but any of them can > fail, then I want the bottom part done. I wonder where everyone eas when I asked this question a lot of time ago. I said I thought the behavior should be like I described, and no one objected. Personally I think it would be a mistake to allow the COMMIT for the subtransaction to ignore the fact that the subxact was aborted. However I realize what you are proposing, and maybe this can be implemented using a parameter to COMMIT (indicating to not propagate the error if it's in aborted state, but commit normally otherwise). However if everyone disagrees, I can take that part out, and the code would be simpler. IMHO however, it would be less reliable. > In my logic, the subtransaction COMMIT is part of the subtransaction and > should not affect the outer transaction's state. In some cases yes, but not all. In others, the outer transaction could trust that the inner one worked; to make the example you posted work, I'd use a program rather than a script, and check the return values (or the transaction state). If the subxact is in aborted state, issue ROLLBACK and try again; if not, commit. > Unfortunately, we don't have any similar behavior in our 7.4 code > because whether you issue COMMIT or ABORT, it does not affect the outer > session. Of course. This is new functionality. > Right now I think just posting examples will work fine. I think the > above case shows we are not ready for documentation yet. What I would > like is for folks to focus on testing so we can find any open issues > like this one and address them. Ok. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "In Europe they call me Niklaus Wirth; in the US they call me Nickel's worth. That's because in Europe they call me by name, and in the US by value!"
Alvaro Herrera wrote: > On Fri, May 28, 2004 at 01:43:16PM -0400, Bruce Momjian wrote: > > > In this case, I want to try all of the inserts, but any of them can > > fail, then I want the bottom part done. > > I wonder where everyone eas when I asked this question a lot of time > ago. I said I thought the behavior should be like I described, and no > one objected. Sorry, I didn't understand the question at the time, or wasn't paying attention. > Personally I think it would be a mistake to allow the COMMIT for the > subtransaction to ignore the fact that the subxact was aborted. However > I realize what you are proposing, and maybe this can be implemented > using a parameter to COMMIT (indicating to not propagate the error if > it's in aborted state, but commit normally otherwise). > > However if everyone disagrees, I can take that part out, and the code > would be simpler. IMHO however, it would be less reliable. Imagine this case used in a script: BEGIN;DROP TABLE test;CREATE TABLE test(x int);COMMIT; This will not work because the drop might fail. However you could use this: BEGIN;BEGIN;DROP TABLE test;COMMIT;CREATE TABLE test(x int);COMMIT; It is done in a transaction so the table replace is an atomic operation. One interesting idea would be for COMMIT to affect the outer transaction, and END not affect the outer transaction. Of course that kills the logic that COMMIT and END are the same, but it is an interesting idea, and doesn't affect backward compatibility because END/COMMIT behave the same in non-nested transactions. If this is the type of issue we are dealing with for the patch, I feel very good. Good job Alvaro. -- 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
Alvaro Herrera wrote: > On Fri, May 28, 2004 at 04:51:07PM +0900, Tatsuo Ishii wrote: > > > 2) certain behavior was different from what I expected (please correct me > > if my expectation is wrong). > > Yes, the expected behavior is different: if an aborted subtransaction is > closed with a COMMIT or END command, the parent transaction is aborted > too. This is to inhibit an application which blindly expects the > subtransaction to succeed to reach an invalid state. If you want to > return to non-aborted state, end the subtransaction with ROLLBACK > instead. I am interested to know if people agree with this behavior, reproduced below: test=# begin;BEGINtest=# insert into t1 values(1);INSERT 17216 1test=# begin;BEGINtest=# aaa;ERROR: syntax error at or near"aaa" at character 1ERROR: syntax error at or near "aaa" at character 1LINE 1: aaa; ^test=# end;COMMITtest=#select * from t1; <-- I thought this should work sincesubtransaction was closedERROR: current transactionis aborted, commands ignored until end oftransaction blockERROR: current transaction is aborted, commands ignoreduntil end oftransaction blocktest=# end;COMMIT The problem I see with the behavior shown is that there is no way to use subtransactions in scripts, where the queries can't be changed. Consider this: BEGIN;DO SOME WORK...BEGIN;INSERT ...COMMIT;BEGIN;INSERT ...COMMIT;BEGIN;INSERT ...COMMIT;BEGIN;INSERT ...COMMIT;DO MOREWORK...COMMIT; In this case, I want to try all of the inserts, but any of them can fail, then I want the bottom part done. I guess my big question is that if they issue a COMMIT for a subtransaction that failed, do we assume they made a mistake and fail the outer transaction, or do we just accept it and not affect the outer transaction. In my logic, the subtransaction COMMIT is part of the subtransaction and should not affect the outer transaction's state. Unfortunately, we don't have any similar behavior in our 7.4 code because whether you issue COMMIT or ABORT, it does not affect the outer session. Do any other databases have nested transactions, and how to they handle it? I think we should issue a warning but not affect the outer transaction. > But Bob Henkel and you are right: there needs to be documentation. > Initially I figured I could do that later because there is no new > syntax, but it is obviously needed to explain all sorts of assumptions > and behavior like this. Right now I think just posting examples will work fine. I think the above case shows we are not ready for documentation yet. What I would like is for folks to focus on testing so we can find any open issues like this one and address them. -- 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
On Wed, 26 May 2004, Alvaro Herrera wrote: > I have tested it and it passes all regression tests (including ones I > added), plus some more tests I threw at it mainly for concurrency. > Everything behaves as expected. At this time I'd like to have it > reviewed by the critic eye of the committers, and tested by whoever > would be using it. > > I'm open for comments and suggestions and general input. Thank you. I unfortunately didn't really follow the discussions in the past (sorry :( ), but are the transaction state modifying statements done in a subtransaction supposed to live beyond subtransaction rollback? For example, sszabo=# begin; BEGIN sszabo=# begin; BEGIN sszabo=# set transaction read only; SET sszabo=# select * from a;a ---6 (1 row) sszabo=# rollback; ROLLBACK sszabo=# update a set a=6; ERROR: transaction is read-only
On Fri, May 28, 2004 at 05:43:41PM -0700, Stephan Szabo wrote: > On Wed, 26 May 2004, Alvaro Herrera wrote: > > > I have tested it and it passes all regression tests (including ones I > > added), plus some more tests I threw at it mainly for concurrency. > > Everything behaves as expected. At this time I'd like to have it > > reviewed by the critic eye of the committers, and tested by whoever > > would be using it. > > I unfortunately didn't really follow the discussions in the past (sorry :( > ), but are the transaction state modifying statements done in a > subtransaction supposed to live beyond subtransaction rollback? Hmm, I suppose not. I think this applies to all GUC variables, but I wonder if we want to save the value of each one at subtransaction start and recover it at abort? Things could easily get huge. Maybe only saving the ones that are different from the default value, and from the last saved value. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Cómo ponemos nuestros dedos en la arcilla del otro. Eso es la amistad; jugar al alfarero y ver qué formas se pueden sacar del otro" (C. Halloway en La Feria de las Tinieblas, R. Bradbury)
Alvaro Herrera wrote: > On Fri, May 28, 2004 at 05:43:41PM -0700, Stephan Szabo wrote: > > > On Wed, 26 May 2004, Alvaro Herrera wrote: > > > > > I have tested it and it passes all regression tests (including ones I > > > added), plus some more tests I threw at it mainly for concurrency. > > > Everything behaves as expected. At this time I'd like to have it > > > reviewed by the critic eye of the committers, and tested by whoever > > > would be using it. > > > > I unfortunately didn't really follow the discussions in the past (sorry :( > > ), but are the transaction state modifying statements done in a > > subtransaction supposed to live beyond subtransaction rollback? > > Hmm, I suppose not. > > I think this applies to all GUC variables, but I wonder if we want to > save the value of each one at subtransaction start and recover it at > abort? Things could easily get huge. Maybe only saving the ones that > are different from the default value, and from the last saved value. We have an on-commit field in the guc structures to handle commit/rollback settings. Do we need to extend that to subtransactions? I don't think you can save off only the defaults in an efficient manner. -- 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
On Fri, May 28, 2004 at 11:11:27PM -0400, Bruce Momjian wrote: > Alvaro Herrera wrote: > > I think this applies to all GUC variables, but I wonder if we want to > > save the value of each one at subtransaction start and recover it at > > abort? Things could easily get huge. Maybe only saving the ones that > > are different from the default value, and from the last saved value. > > We have an on-commit field in the guc structures to handle > commit/rollback settings. Do we need to extend that to subtransactions? Yes IMHO. I'm not sure actually _how_ should this be handled. Maybe the on-commit field should go away and be replaced by something more general (probably a stack, like everything else). Let me look at the code. > I don't think you can save off only the defaults in an efficient manner. What do you mean by efficient? Space efficient? It will be much more efficient to save only the changed values. Time efficient? We will have to traverse the whole list anyway, whether we only save the changed values or all of them. Remember, we already traverse the whole list of shared buffers, the whole CatCache, the whole Relcache, maybe do some repallocs, and lots of other stuff. Traversing the whole GUC array does not seem all that expensive to me ... After all, we are saving lots of I/O if subxacts are used correctly (try, rollback, try again -- you save the XLog for the first try.) -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Oh, great altar of passive entertainment, bestow upon me thy discordant images at such speed as to render linear thought impossible" (Calvin a la TV)
Alvaro Herrera wrote: > On Fri, May 28, 2004 at 11:11:27PM -0400, Bruce Momjian wrote: > > Alvaro Herrera wrote: > > > > I think this applies to all GUC variables, but I wonder if we want to > > > save the value of each one at subtransaction start and recover it at > > > abort? Things could easily get huge. Maybe only saving the ones that > > > are different from the default value, and from the last saved value. > > > > We have an on-commit field in the guc structures to handle > > commit/rollback settings. Do we need to extend that to subtransactions? > > Yes IMHO. I'm not sure actually _how_ should this be handled. Maybe > the on-commit field should go away and be replaced by something more > general (probably a stack, like everything else). Let me look at the > code. > > > I don't think you can save off only the defaults in an efficient manner. > > What do you mean by efficient? Space efficient? It will be much more > efficient to save only the changed values. Time efficient? We will > have to traverse the whole list anyway, whether we only save the changed > values or all of them. > > Remember, we already traverse the whole list of shared buffers, the > whole CatCache, the whole Relcache, maybe do some repallocs, and lots of > other stuff. Traversing the whole GUC array does not seem all that > expensive to me ... After all, we are saving lots of I/O if subxacts > are used correctly (try, rollback, try again -- you save the XLog for > the first try.) My comment was based on the fact that guc already does some special _saves_ when you change a value and triggers some stuff on xact end. I was just thinking it would be cleaner to use that infrastructure rather than do a scan not knowing if any GUC will change or not, but if a scan is easier, I think that would be fine. -- 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
On Sat, May 29, 2004 at 08:25:27AM -0700, Stephan Szabo wrote: > Also related, although START TRANSACTION (specifying isolation level or > read onlyness as part) is currently defined to act as if set transaction > was used, it seems really odd that the settings would leak to the outer > translation even on a commit and that you can't specify isolation level - > even if it's the same level - if the outer transaction has done any > queries. Hmm ... isolation level and read onlyness was discussed last year and I think we had a working design. I'll look into my archives. > BTW: For the deferred trigger stuff, I am guessing you haven't touched > that at all in the current patch? > > I wonder if the following would work assuming that we want deferred > triggers to run at outer transaction end? Ah, this seems to work. I'll implement it and I'll let you know how it goes. > I think it might be possible to do the queue deallocation for > subtransaction abort with appropriate context work (each one gets a > context under its parent's and on abort it's removed and on commit it's > not until you reach the outermost?) but I haven't though about it enough. Actually there already is such a global context and I think it's appropiate here. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Ninguna manada de bestias tiene una voz tan horrible como la humana" (Orual)
On Sat, 29 May 2004, Alvaro Herrera wrote: > On Sat, May 29, 2004 at 08:25:27AM -0700, Stephan Szabo wrote: > > > Also related, although START TRANSACTION (specifying isolation level or > > read onlyness as part) is currently defined to act as if set transaction > > was used, it seems really odd that the settings would leak to the outer > > translation even on a commit and that you can't specify isolation level - > > even if it's the same level - if the outer transaction has done any > > queries. > > Hmm ... isolation level and read onlyness was discussed last year and I > think we had a working design. I'll look into my archives. > > > > BTW: For the deferred trigger stuff, I am guessing you haven't touched > > that at all in the current patch? > > > > I wonder if the following would work assuming that we want deferred > > triggers to run at outer transaction end? > > Ah, this seems to work. I'll implement it and I'll let you know how it > goes. Note, that I think you'd still need to do some kind of tiered thing for set constraint status, but I didn't think about that yet. > > I think it might be possible to do the queue deallocation for > > subtransaction abort with appropriate context work (each one gets a > > context under its parent's and on abort it's removed and on commit it's > > not until you reach the outermost?) but I haven't though about it enough. > > Actually there already is such a global context and I think it's appropiate > here. We probably want to be able to easily remove all the items for an aborted sub transaction, and I was hoping that there might be an easy way to use the contexts to do that rather than manually trolling through deallocating entries from the queue.
On Fri, 28 May 2004, Alvaro Herrera wrote: > On Fri, May 28, 2004 at 05:43:41PM -0700, Stephan Szabo wrote: > > > On Wed, 26 May 2004, Alvaro Herrera wrote: > > > > > I have tested it and it passes all regression tests (including ones I > > > added), plus some more tests I threw at it mainly for concurrency. > > > Everything behaves as expected. At this time I'd like to have it > > > reviewed by the critic eye of the committers, and tested by whoever > > > would be using it. > > > > I unfortunately didn't really follow the discussions in the past (sorry :( > > ), but are the transaction state modifying statements done in a > > subtransaction supposed to live beyond subtransaction rollback? > > Hmm, I suppose not. Actually, looking at it, for set transaction, the worse one is probably isolation level because it can't be set after a query has been run and I'm not entirely sure what the behavior should look like if you did something like: begin;set transaction isolation level serializable;begin; set transaction isolation level read committed; select * from a;rollback; At that point, is the transaction in serializable method and is the snapshot set even though the select "never happened" and what does the above really mean. Is it that the results of that first transactions are out of serializability with the rest of the transaction? Also related, although START TRANSACTION (specifying isolation level or read onlyness as part) is currently defined to act as if set transaction was used, it seems really odd that the settings would leak to the outer translation even on a commit and that you can't specify isolation level - even if it's the same level - if the outer transaction has done any queries. ----- BTW: For the deferred trigger stuff, I am guessing you haven't touched that at all in the current patch? I wonder if the following would work assuming that we want deferred triggers to run at outer transaction end? -- There's one trigger queue list and a stack of "last event before this (sub)transaction started" pointers in and a current one similar to _imm is the one for the current subtransaction. Scanning the list takes an extra argument to say whether to consolidate the list or not (generally true except for set state). On outer transaction start,NULL is set as the last event at start pointer On subtransaction start,The current last event at start pointer is added to the stack.The last event at start pointer isset to _imm.The _imm pointer should already be in the correct place On subtransaction abort,The queue is deallocated after the last event at start pointer and the event in question its nextpointer reset. (if null, the list is entirely deallocated and the list is set as null)The _imm pointer is set to thelast event at start pointer.The last event at start pointer is popped from the stack. On outer transaction abort,Clear the queue. On subtransaction commit,The last event at start pointer is popped from the stack. On outer transaction commitScan the deferred events. On set state,We call DeferredTriggerInvokeEvents with (true, false) rather than relying on the statement end to do it (as true, true). This should set the _imm pointer so that the end of statement won't scan anything anyway. -- I think it might be possible to do the queue deallocation for subtransaction abort with appropriate context work (each one gets a context under its parent's and on abort it's removed and on commit it's not until you reach the outermost?) but I haven't though about it enough. And I think the non-removing events on set transaction could be fixed as well.
On Sat, 29 May 2004, Alvaro Herrera wrote: > On Sat, May 29, 2004 at 08:25:27AM -0700, Stephan Szabo wrote: > > > BTW: For the deferred trigger stuff, I am guessing you haven't touched > > that at all in the current patch? > > > > I wonder if the following would work assuming that we want deferred > > triggers to run at outer transaction end? > > Ah, this seems to work. I'll implement it and I'll let you know how it > goes. Ugh... There's one further wrinkle I hadn't thought about, imagine the following: begin;-- here the transaction does something that makes deferred trigger-- entriesbegin; set constraints all immediate; --we now run through doing the deferred trigger itemsrollback;-- we need to unmark that the deferred items from the-- outertransaction have been run. However, in general,-- it might not be all entries nor all entries that are marked-- asdone. I'm not sure how expensive it is to check if a given subxact has committed, but maybe instead of just done/not done, we need to say something like what xid marked the trigger and instead of if (!(event->dte_event & (TRIGGER_DEFERRED_DONE | TRIGGER_DEFERRED_CANCELED))) inside deferredTriggerInvokeEvents we do something like: if (!((event->dte_event & (TRIGGER_DEFERRED_DONE | TRIGGER_DEFERRED_CANCELED)) && /*something to check that the marking xact is either myself or a committed subxact*/))
On 5/28/2004 2:52 PM, Alvaro Herrera wrote: > On Fri, May 28, 2004 at 01:43:16PM -0400, Bruce Momjian wrote: > >> In this case, I want to try all of the inserts, but any of them can >> fail, then I want the bottom part done. > > I wonder where everyone eas when I asked this question a lot of time > ago. I said I thought the behavior should be like I described, and no > one objected. > > Personally I think it would be a mistake to allow the COMMIT for the > subtransaction to ignore the fact that the subxact was aborted. However > I realize what you are proposing, and maybe this can be implemented > using a parameter to COMMIT (indicating to not propagate the error if > it's in aborted state, but commit normally otherwise). I agree on this one. Subtransactions are a feature to add more fine control to applications, not to ignore error checking for scripting. > > However if everyone disagrees, I can take that part out, and the code > would be simpler. IMHO however, it would be less reliable. Please don't. Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #
This may be out of scope but I'm goign to mention it. Would error trapping
help any of these issues. In Oracle PL/SQL you have an exception section to
handle any known or unknown errors. Is this for the future or does the
nested xacts code include this at all?
|---------+---------------------------------->
| | Bruce Momjian |
| | <pgman@candle.pha.pa.us|
| | > |
| | Sent by: |
| | pgsql-hackers-owner@pos|
| | tgresql.org |
| | |
| | |
| | 05/28/2004 03:05 PM |
| | |
|---------+---------------------------------->
>------------------------------------------------------------------------------------------------------------------------------|
|
| | To: Alvaro Herrera <alvherre@dcc.uchile.cl>
| | cc: Tatsuo Ishii <t-ishii@sra.co.jp>, pgsql-hackers@postgresql.org
| | Subject: Re: [HACKERS] Nested xacts: looking for testers and review
|
>------------------------------------------------------------------------------------------------------------------------------|
Alvaro Herrera wrote:
> On Fri, May 28, 2004 at 01:43:16PM -0400, Bruce Momjian wrote:
>
> > In this case, I want to try all of the inserts, but any of them can
> > fail, then I want the bottom part done.
>
> I wonder where everyone eas when I asked this question a lot of time
> ago. I said I thought the behavior should be like I described, and no
> one objected.
Sorry, I didn't understand the question at the time, or wasn't paying
attention.
> Personally I think it would be a mistake to allow the COMMIT for the
> subtransaction to ignore the fact that the subxact was aborted. However
> I realize what you are proposing, and maybe this can be implemented
> using a parameter to COMMIT (indicating to not propagate the error if
> it's in aborted state, but commit normally otherwise).
>
> However if everyone disagrees, I can take that part out, and the code
> would be simpler. IMHO however, it would be less reliable.
Imagine this case used in a script:
BEGIN; DROP TABLE test; CREATE TABLE test(x int); COMMIT;
This will not work because the drop might fail. However you could use
this:
BEGIN; BEGIN; DROP TABLE test; COMMIT; CREATE TABLE test(x
int); COMMIT;
It is done in a transaction so the table replace is an atomic operation.
One interesting idea would be for COMMIT to affect the outer
transaction, and END not affect the outer transaction. Of course that
kills the logic that COMMIT and END are the same, but it is an
interesting idea, and doesn't affect backward compatibility because
END/COMMIT behave the same in non-nested transactions.
If this is the type of issue we are dealing with for the patch, I feel
very good. Good job Alvaro.
-- 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 6: Have you searched our list archives?
http://archives.postgresql.org
*************************************************************************
PRIVILEGED AND CONFIDENTIAL: This communication, including attachments, is for the exclusive use of addressee and may
containproprietary, confidential and/or privileged information. If you are not the intended recipient, any use,
copying,disclosure, dissemination or distribution is strictly prohibited. If you are not the intended recipient,
pleasenotify the sender immediately by return e-mail, delete this communication and destroy all copies.
*************************************************************************
On Sun, May 30, 2004 at 04:07:27AM -0700, Stephan Szabo wrote: > On Sat, 29 May 2004, Alvaro Herrera wrote: > > Ah, this seems to work. I'll implement it and I'll let you know how it > > goes. > > Ugh... There's one further wrinkle I hadn't thought about, imagine the > following: Ok Stephan, thank you very much for your help. I implemented this and it seems to work, at least on my (limited?) test scenario. If you have some spare time I'd like you to test it and see if you can break it (I posted it to -patches yesterday). Or please see my test case below. Is it missing something? Note that if I take out any of the four inserts just before the end of the transaction, the whole thing is rejected. DROP TABLE foo CASCADE; DROP TABLE bar CASCADE; CREATE TABLE foo (A INT UNIQUE); CREATE TABLE bar (A INT REFERENCES foo(A) DEFERRABLE); DELETE FROM bar; DELETE FROM foo; INSERT INTO foo VALUES (1); INSERT INTO foo VALUES (2); BEGIN;SET CONSTRAINTS ALL DEFERRED;INSERT INTO bar VALUES (1);BEGIN; INSERT INTO bar VALUES (3);COMMIT;BEGIN; BEGIN; INSERT INTO bar VALUES (4); COMMIT; INSERT INTO foo VALUES (3); SET CONSTRAINTS ALL IMMEDIATE;ROLLBACK;SETCONSTRAINTS ALL DEFERRED;BEGIN; INSERT INTO bar VALUES (5);COMMIT;BEGIN; BEGIN; INSERTINTO bar VALUES (6); ROLLBACK;COMMIT;BEGIN; INSERT INTO bar VALUES (7);COMMIT;BEGIN; BEGIN; INSERTINTO bar VALUES (9); COMMIT;COMMIT;INSERT INTO foo VALUES(3);INSERT INTO foo VALUES(5);INSERT INTO foo VALUES(7);INSERTINTO foo VALUES(9); COMMIT; -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Acepta los honores y aplausos y perderás tu libertad"
On Fri, May 28, 2004 at 04:05:40PM -0400, Bruce Momjian wrote:
Bruce,
> One interesting idea would be for COMMIT to affect the outer
> transaction, and END not affect the outer transaction. Of course that
> kills the logic that COMMIT and END are the same, but it is an
> interesting idea, and doesn't affect backward compatibility because
> END/COMMIT behave the same in non-nested transactions.
I implemented this behavior by using parameters to COMMIT/END. I didn't
want to add new keywords to the grammar so I just picked up
"COMMIT WITHOUT ABORT". (Originally I had thought "COMMIT IGNORE
ERRORS" but those would be two new keywords and I don't want to mess
around with the grammar. If there are different opinions, tweaking the
grammar is easy).
So the behavior I originally implemented is still there:
alvherre=# begin;
BEGIN
alvherre=# begin;
BEGIN
alvherre=# select foo;
ERROR: no existe la columna "foo"
alvherre=# commit;
COMMIT
alvherre=# select 1;
ERROR: transacción abortada, las consultas serán ignoradas hasta el fin de bloque de transacción
alvherre=# commit;
COMMIT
However if one wants to use in script the behavior you propose, use
the following:
alvherre=# begin;
BEGIN
alvherre=# begin;
BEGIN
alvherre=# select foo;
ERROR: no existe la columna "foo"
alvherre=# commit without abort;
COMMIT
alvherre=# select 1;
?column?
----------
1
(1 fila)
alvherre=# commit;
COMMIT
The patch is attached. It applies only after the previous patch,
obviously.
--
Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
"Ciencias políticas es la ciencia de entender por qué
los políticos actúan como lo hacen" (netfunny.com)
Attachment
Alvaro Herrera wrote: > > One interesting idea would be for COMMIT to affect the outer > > transaction, and END not affect the outer transaction. Of course that > > kills the logic that COMMIT and END are the same, but it is an > > interesting idea, and doesn't affect backward compatibility because > > END/COMMIT behave the same in non-nested transactions. > > I implemented this behavior by using parameters to COMMIT/END. I didn't > want to add new keywords to the grammar so I just picked up > "COMMIT WITHOUT ABORT". (Originally I had thought "COMMIT IGNORE > ERRORS" but those would be two new keywords and I don't want to mess > around with the grammar. If there are different opinions, tweaking the > grammar is easy). > > So the behavior I originally implemented is still there: > > alvherre=# begin; > BEGIN > alvherre=# begin; > BEGIN > alvherre=# select foo; > ERROR: no existe la columna "foo" > alvherre=# commit; > COMMIT > alvherre=# select 1; > ERROR: transacci?n abortada, las consultas ser?n ignoradas hasta el fin de bloque de transacci?n > alvherre=# commit; > COMMIT Perfect. Your suggested behavior is best. I think I like "COMMIT IGNORE ABORT" best, but we can disucss this some more. -- 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
On Wed, 9 Jun 2004, Alvaro Herrera wrote: > On Sun, May 30, 2004 at 04:07:27AM -0700, Stephan Szabo wrote: > > On Sat, 29 May 2004, Alvaro Herrera wrote: > > > > Ah, this seems to work. I'll implement it and I'll let you know how it > > > goes. > > > > Ugh... There's one further wrinkle I hadn't thought about, imagine the > > following: > > Ok Stephan, thank you very much for your help. I implemented this and > it seems to work, at least on my (limited?) test scenario. If you have > some spare time I'd like you to test it and see if you can break it (I > posted it to -patches yesterday). Unfortunately, I've gotten it to fail, but I haven't looked in depth (I'm at work, so I'm doing it during compilations and such.) I made a file ins with a 1000000 line copy to a table named fk with the value 1, and then if I do the following: create table pk(a int primary key); create table fk(a int references pk(a) initially deferred); insert into pk values (1); begin; begin; \i ins \i ins rollback; \i ins commit; It appears to exception on the third \i ins with the tail_thisxact pointing into a bad place.
On Wed, 9 Jun 2004, Stephan Szabo wrote: > On Wed, 9 Jun 2004, Alvaro Herrera wrote: > > > On Sun, May 30, 2004 at 04:07:27AM -0700, Stephan Szabo wrote: > > > On Sat, 29 May 2004, Alvaro Herrera wrote: > > > > > > Ah, this seems to work. I'll implement it and I'll let you know how it > > > > goes. > > > > > > Ugh... There's one further wrinkle I hadn't thought about, imagine the > > > following: > > > > Ok Stephan, thank you very much for your help. I implemented this and > > it seems to work, at least on my (limited?) test scenario. If you have > > some spare time I'd like you to test it and see if you can break it (I > > posted it to -patches yesterday). > > Unfortunately, I've gotten it to fail, but I haven't looked in depth (I'm > at work, so I'm doing it during compilations and such.) > > I made a file ins with a 1000000 line copy to a table named fk with the > value 1, and then if I do the following: > > create table pk(a int primary key); > create table fk(a int references pk(a) initially deferred); > insert into pk values (1); > > begin; > begin; > \i ins > \i ins > rollback; > \i ins > commit; > > It appears to exception on the third \i ins with the tail_thisxact > pointing into a bad place. Okay - I think I see what's going on here. It looks like deferredTriggerInvokeEvents is being run (immediate_only), but since deferredTriggers->events_imm is NULL it's using deferredTriggers->events as the start of the list to check, but this value isn't getting reset in DeferredTriggerEndSubXact in the case that the entire list was created in an aborted subtransaction.
Am I the only one who has a hard time understanding why COMMIT in the
case of an error is allowed? Since nothing is actually committed, but
instead everything was actually rolled back. Isn't it misleading to
allow a commit under these circumstances?
Then to further extend the commit syntax with COMMIT WITHOUT ABORT makes
even less since, IMHO. If we are going to extend the syntax shouldn't
we be extending ROLLBACK or END, something other than COMMIT so that we
don't imply that anything was actually committed.
Perhaps I am being too literal here in reading the keyword COMMIT as
meaning that something was actually committed, instead of COMMIT simply
being end-of-transaction that may or may not have committed the changes
in that transaction. I have always looked at COMMIT and ROLLBACK as a
symmetric pair of commands - ROLLBACK -> the changes in the transaction
are not committed, COMMIT -> the changes in the transaction are
committed. That symmetry doesn't exist in reality since COMMIT only
means that the changes might have been committed.
--Barry
Alvaro Herrera wrote:
> On Fri, May 28, 2004 at 04:05:40PM -0400, Bruce Momjian wrote:
>
> Bruce,
>
>
>>One interesting idea would be for COMMIT to affect the outer
>>transaction, and END not affect the outer transaction. Of course that
>>kills the logic that COMMIT and END are the same, but it is an
>>interesting idea, and doesn't affect backward compatibility because
>>END/COMMIT behave the same in non-nested transactions.
>
>
> I implemented this behavior by using parameters to COMMIT/END. I didn't
> want to add new keywords to the grammar so I just picked up
> "COMMIT WITHOUT ABORT". (Originally I had thought "COMMIT IGNORE
> ERRORS" but those would be two new keywords and I don't want to mess
> around with the grammar. If there are different opinions, tweaking the
> grammar is easy).
>
> So the behavior I originally implemented is still there:
>
> alvherre=# begin;
> BEGIN
> alvherre=# begin;
> BEGIN
> alvherre=# select foo;
> ERROR: no existe la columna "foo"
> alvherre=# commit;
> COMMIT
> alvherre=# select 1;
> ERROR: transacción abortada, las consultas serán ignoradas hasta el fin de bloque de transacción
> alvherre=# commit;
> COMMIT
>
>
> However if one wants to use in script the behavior you propose, use
> the following:
>
> alvherre=# begin;
> BEGIN
> alvherre=# begin;
> BEGIN
> alvherre=# select foo;
> ERROR: no existe la columna "foo"
> alvherre=# commit without abort;
> COMMIT
> alvherre=# select 1;
> ?column?
> ----------
> 1
> (1 fila)
>
> alvherre=# commit;
> COMMIT
>
>
> The patch is attached. It applies only after the previous patch,
> obviously.
>
>
>
> ------------------------------------------------------------------------
>
> diff -Ncr --exclude-from=diff-ignore 10bgwriter/src/backend/access/transam/xact.c
13commitOpt/src/backend/access/transam/xact.c
> *** 10bgwriter/src/backend/access/transam/xact.c 2004-06-08 17:34:49.000000000 -0400
> --- 13commitOpt/src/backend/access/transam/xact.c 2004-06-09 12:00:49.000000000 -0400
> ***************
> *** 2125,2131 ****
> * EndTransactionBlock
> */
> void
> ! EndTransactionBlock(void)
> {
> TransactionState s = CurrentTransactionState;
>
> --- 2125,2131 ----
> * EndTransactionBlock
> */
> void
> ! EndTransactionBlock(bool ignore)
> {
> TransactionState s = CurrentTransactionState;
>
> ***************
> *** 2163,2172 ****
> /*
> * here we are in an aborted subtransaction. Signal
> * CommitTransactionCommand() to clean up and return to the
> ! * parent transaction.
> */
> case TBLOCK_SUBABORT:
> ! s->blockState = TBLOCK_SUBENDABORT_ERROR;
> break;
>
> case TBLOCK_STARTED:
> --- 2163,2177 ----
> /*
> * here we are in an aborted subtransaction. Signal
> * CommitTransactionCommand() to clean up and return to the
> ! * parent transaction. If we are asked to ignore the errors
> ! * in the subtransaction, the parent can continue; else,
> ! * it has to be put in aborted state too.
> */
> case TBLOCK_SUBABORT:
> ! if (ignore)
> ! s->blockState = TBLOCK_SUBENDABORT_OK;
> ! else
> ! s->blockState = TBLOCK_SUBENDABORT_ERROR;
> break;
>
> case TBLOCK_STARTED:
> diff -Ncr --exclude-from=diff-ignore 10bgwriter/src/backend/parser/gram.y 13commitOpt/src/backend/parser/gram.y
> *** 10bgwriter/src/backend/parser/gram.y 2004-06-03 20:46:48.000000000 -0400
> --- 13commitOpt/src/backend/parser/gram.y 2004-06-09 11:51:04.000000000 -0400
> ***************
> *** 225,232 ****
> target_list update_target_list insert_column_list
> insert_target_list def_list opt_indirection
> group_clause TriggerFuncArgs select_limit
> ! opt_select_limit opclass_item_list transaction_mode_list
> ! transaction_mode_list_or_empty
> TableFuncElementList
> prep_type_clause prep_type_list
> execute_param_clause
> --- 225,232 ----
> target_list update_target_list insert_column_list
> insert_target_list def_list opt_indirection
> group_clause TriggerFuncArgs select_limit
> ! opt_select_limit opclass_item_list transaction_commit_opts
> ! transaction_mode_list transaction_mode_list_or_empty
> TableFuncElementList
> prep_type_clause prep_type_list
> execute_param_clause
> ***************
> *** 3765,3782 ****
> n->options = $3;
> $$ = (Node *)n;
> }
> ! | COMMIT opt_transaction
> {
> TransactionStmt *n = makeNode(TransactionStmt);
> n->kind = TRANS_STMT_COMMIT;
> ! n->options = NIL;
> $$ = (Node *)n;
> }
> ! | END_P opt_transaction
> {
> TransactionStmt *n = makeNode(TransactionStmt);
> n->kind = TRANS_STMT_COMMIT;
> ! n->options = NIL;
> $$ = (Node *)n;
> }
> | ROLLBACK opt_transaction
> --- 3765,3782 ----
> n->options = $3;
> $$ = (Node *)n;
> }
> ! | COMMIT opt_transaction transaction_commit_opts
> {
> TransactionStmt *n = makeNode(TransactionStmt);
> n->kind = TRANS_STMT_COMMIT;
> ! n->options = $3;
> $$ = (Node *)n;
> }
> ! | END_P opt_transaction transaction_commit_opts
> {
> TransactionStmt *n = makeNode(TransactionStmt);
> n->kind = TRANS_STMT_COMMIT;
> ! n->options = $3;
> $$ = (Node *)n;
> }
> | ROLLBACK opt_transaction
> ***************
> *** 3827,3832 ****
> --- 3827,3841 ----
> | READ WRITE { $$ = FALSE; }
> ;
>
> + transaction_commit_opts:
> + WITHOUT ABORT_P
> + {
> + $$ = list_make1(makeDefElem("ignore_errors",
> + makeBoolConst(true, false)));
> + }
> + | /* EMPTY */
> + { $$ = NIL; }
> + ;
>
> /*****************************************************************************
> *
> diff -Ncr --exclude-from=diff-ignore 10bgwriter/src/backend/tcop/utility.c 13commitOpt/src/backend/tcop/utility.c
> *** 10bgwriter/src/backend/tcop/utility.c 2004-05-29 19:20:14.000000000 -0400
> --- 13commitOpt/src/backend/tcop/utility.c 2004-06-09 12:02:53.000000000 -0400
> ***************
> *** 351,357 ****
> break;
>
> case TRANS_STMT_COMMIT:
> ! EndTransactionBlock();
> break;
>
> case TRANS_STMT_ROLLBACK:
> --- 351,374 ----
> break;
>
> case TRANS_STMT_COMMIT:
> ! {
> ! bool ignore = false;
> !
> ! if (stmt->options)
> ! {
> ! ListCell *head;
> !
> ! foreach(head, stmt->options)
> ! {
> ! DefElem *item = (DefElem *) lfirst(head);
> !
> ! if (strcmp(item->defname, "ignore_errors") == 0)
> ! ignore = true;
> ! }
> ! }
> !
> ! EndTransactionBlock(ignore);
> ! }
> break;
>
> case TRANS_STMT_ROLLBACK:
> diff -Ncr --exclude-from=diff-ignore 10bgwriter/src/include/access/xact.h 13commitOpt/src/include/access/xact.h
> *** 10bgwriter/src/include/access/xact.h 2004-06-08 17:48:36.000000000 -0400
> --- 13commitOpt/src/include/access/xact.h 2004-06-09 12:00:19.000000000 -0400
> ***************
> *** 171,177 ****
> extern void CommitTransactionCommand(void);
> extern void AbortCurrentTransaction(void);
> extern void BeginTransactionBlock(void);
> ! extern void EndTransactionBlock(void);
> extern bool IsSubTransaction(void);
> extern bool IsTransactionBlock(void);
> extern bool IsTransactionOrTransactionBlock(void);
> --- 171,177 ----
> extern void CommitTransactionCommand(void);
> extern void AbortCurrentTransaction(void);
> extern void BeginTransactionBlock(void);
> ! extern void EndTransactionBlock(bool ignore);
> extern bool IsSubTransaction(void);
> extern bool IsTransactionBlock(void);
> extern bool IsTransactionOrTransactionBlock(void);
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings
Well, the default behavior of COMMIT for an aborted subtransaction is
that it will abort the upper transaction too, so I think this is the
behavior you want.
We are considering allowing COMMIT IGNORE ABORT for scripts that want to
do a subtransaction, but don't care if it fails, and because it is a
script, they can't test the return value to send ROLLBACK:
BEGIN;BEGIN;DROP TABLE test;COMMITCREATE TABLE test(x int);COMMIT;
In this case you don't care if the DROP fails, but you do it all in a
subtransaction so the visibility happens all at once.
---------------------------------------------------------------------------
Barry Lind wrote:
> Am I the only one who has a hard time understanding why COMMIT in the
> case of an error is allowed? Since nothing is actually committed, but
> instead everything was actually rolled back. Isn't it misleading to
> allow a commit under these circumstances?
>
> Then to further extend the commit syntax with COMMIT WITHOUT ABORT makes
> even less since, IMHO. If we are going to extend the syntax shouldn't
> we be extending ROLLBACK or END, something other than COMMIT so that we
> don't imply that anything was actually committed.
>
> Perhaps I am being too literal here in reading the keyword COMMIT as
> meaning that something was actually committed, instead of COMMIT simply
> being end-of-transaction that may or may not have committed the changes
> in that transaction. I have always looked at COMMIT and ROLLBACK as a
> symmetric pair of commands - ROLLBACK -> the changes in the transaction
> are not committed, COMMIT -> the changes in the transaction are
> committed. That symmetry doesn't exist in reality since COMMIT only
> means that the changes might have been committed.
>
> --Barry
>
>
> Alvaro Herrera wrote:
> > On Fri, May 28, 2004 at 04:05:40PM -0400, Bruce Momjian wrote:
> >
> > Bruce,
> >
> >
> >>One interesting idea would be for COMMIT to affect the outer
> >>transaction, and END not affect the outer transaction. Of course that
> >>kills the logic that COMMIT and END are the same, but it is an
> >>interesting idea, and doesn't affect backward compatibility because
> >>END/COMMIT behave the same in non-nested transactions.
> >
> >
> > I implemented this behavior by using parameters to COMMIT/END. I didn't
> > want to add new keywords to the grammar so I just picked up
> > "COMMIT WITHOUT ABORT". (Originally I had thought "COMMIT IGNORE
> > ERRORS" but those would be two new keywords and I don't want to mess
> > around with the grammar. If there are different opinions, tweaking the
> > grammar is easy).
> >
> > So the behavior I originally implemented is still there:
> >
> > alvherre=# begin;
> > BEGIN
> > alvherre=# begin;
> > BEGIN
> > alvherre=# select foo;
> > ERROR: no existe la columna "foo"
> > alvherre=# commit;
> > COMMIT
> > alvherre=# select 1;
> > ERROR: transacci?n abortada, las consultas ser?n ignoradas hasta el fin de bloque de transacci?n
> > alvherre=# commit;
> > COMMIT
> >
> >
> > However if one wants to use in script the behavior you propose, use
> > the following:
> >
> > alvherre=# begin;
> > BEGIN
> > alvherre=# begin;
> > BEGIN
> > alvherre=# select foo;
> > ERROR: no existe la columna "foo"
> > alvherre=# commit without abort;
> > COMMIT
> > alvherre=# select 1;
> > ?column?
> > ----------
> > 1
> > (1 fila)
> >
> > alvherre=# commit;
> > COMMIT
> >
> >
> > The patch is attached. It applies only after the previous patch,
> > obviously.
> >
> >
> >
> > ------------------------------------------------------------------------
> >
> > diff -Ncr --exclude-from=diff-ignore 10bgwriter/src/backend/access/transam/xact.c
13commitOpt/src/backend/access/transam/xact.c
> > *** 10bgwriter/src/backend/access/transam/xact.c 2004-06-08 17:34:49.000000000 -0400
> > --- 13commitOpt/src/backend/access/transam/xact.c 2004-06-09 12:00:49.000000000 -0400
> > ***************
> > *** 2125,2131 ****
> > * EndTransactionBlock
> > */
> > void
> > ! EndTransactionBlock(void)
> > {
> > TransactionState s = CurrentTransactionState;
> >
> > --- 2125,2131 ----
> > * EndTransactionBlock
> > */
> > void
> > ! EndTransactionBlock(bool ignore)
> > {
> > TransactionState s = CurrentTransactionState;
> >
> > ***************
> > *** 2163,2172 ****
> > /*
> > * here we are in an aborted subtransaction. Signal
> > * CommitTransactionCommand() to clean up and return to the
> > ! * parent transaction.
> > */
> > case TBLOCK_SUBABORT:
> > ! s->blockState = TBLOCK_SUBENDABORT_ERROR;
> > break;
> >
> > case TBLOCK_STARTED:
> > --- 2163,2177 ----
> > /*
> > * here we are in an aborted subtransaction. Signal
> > * CommitTransactionCommand() to clean up and return to the
> > ! * parent transaction. If we are asked to ignore the errors
> > ! * in the subtransaction, the parent can continue; else,
> > ! * it has to be put in aborted state too.
> > */
> > case TBLOCK_SUBABORT:
> > ! if (ignore)
> > ! s->blockState = TBLOCK_SUBENDABORT_OK;
> > ! else
> > ! s->blockState = TBLOCK_SUBENDABORT_ERROR;
> > break;
> >
> > case TBLOCK_STARTED:
> > diff -Ncr --exclude-from=diff-ignore 10bgwriter/src/backend/parser/gram.y 13commitOpt/src/backend/parser/gram.y
> > *** 10bgwriter/src/backend/parser/gram.y 2004-06-03 20:46:48.000000000 -0400
> > --- 13commitOpt/src/backend/parser/gram.y 2004-06-09 11:51:04.000000000 -0400
> > ***************
> > *** 225,232 ****
> > target_list update_target_list insert_column_list
> > insert_target_list def_list opt_indirection
> > group_clause TriggerFuncArgs select_limit
> > ! opt_select_limit opclass_item_list transaction_mode_list
> > ! transaction_mode_list_or_empty
> > TableFuncElementList
> > prep_type_clause prep_type_list
> > execute_param_clause
> > --- 225,232 ----
> > target_list update_target_list insert_column_list
> > insert_target_list def_list opt_indirection
> > group_clause TriggerFuncArgs select_limit
> > ! opt_select_limit opclass_item_list transaction_commit_opts
> > ! transaction_mode_list transaction_mode_list_or_empty
> > TableFuncElementList
> > prep_type_clause prep_type_list
> > execute_param_clause
> > ***************
> > *** 3765,3782 ****
> > n->options = $3;
> > $$ = (Node *)n;
> > }
> > ! | COMMIT opt_transaction
> > {
> > TransactionStmt *n = makeNode(TransactionStmt);
> > n->kind = TRANS_STMT_COMMIT;
> > ! n->options = NIL;
> > $$ = (Node *)n;
> > }
> > ! | END_P opt_transaction
> > {
> > TransactionStmt *n = makeNode(TransactionStmt);
> > n->kind = TRANS_STMT_COMMIT;
> > ! n->options = NIL;
> > $$ = (Node *)n;
> > }
> > | ROLLBACK opt_transaction
> > --- 3765,3782 ----
> > n->options = $3;
> > $$ = (Node *)n;
> > }
> > ! | COMMIT opt_transaction transaction_commit_opts
> > {
> > TransactionStmt *n = makeNode(TransactionStmt);
> > n->kind = TRANS_STMT_COMMIT;
> > ! n->options = $3;
> > $$ = (Node *)n;
> > }
> > ! | END_P opt_transaction transaction_commit_opts
> > {
> > TransactionStmt *n = makeNode(TransactionStmt);
> > n->kind = TRANS_STMT_COMMIT;
> > ! n->options = $3;
> > $$ = (Node *)n;
> > }
> > | ROLLBACK opt_transaction
> > ***************
> > *** 3827,3832 ****
> > --- 3827,3841 ----
> > | READ WRITE { $$ = FALSE; }
> > ;
> >
> > + transaction_commit_opts:
> > + WITHOUT ABORT_P
> > + {
> > + $$ = list_make1(makeDefElem("ignore_errors",
> > + makeBoolConst(true, false)));
> > + }
> > + | /* EMPTY */
> > + { $$ = NIL; }
> > + ;
> >
> > /*****************************************************************************
> > *
> > diff -Ncr --exclude-from=diff-ignore 10bgwriter/src/backend/tcop/utility.c 13commitOpt/src/backend/tcop/utility.c
> > *** 10bgwriter/src/backend/tcop/utility.c 2004-05-29 19:20:14.000000000 -0400
> > --- 13commitOpt/src/backend/tcop/utility.c 2004-06-09 12:02:53.000000000 -0400
> > ***************
> > *** 351,357 ****
> > break;
> >
> > case TRANS_STMT_COMMIT:
> > ! EndTransactionBlock();
> > break;
> >
> > case TRANS_STMT_ROLLBACK:
> > --- 351,374 ----
> > break;
> >
> > case TRANS_STMT_COMMIT:
> > ! {
> > ! bool ignore = false;
> > !
> > ! if (stmt->options)
> > ! {
> > ! ListCell *head;
> > !
> > ! foreach(head, stmt->options)
> > ! {
> > ! DefElem *item = (DefElem *) lfirst(head);
> > !
> > ! if (strcmp(item->defname, "ignore_errors") == 0)
> > ! ignore = true;
> > ! }
> > ! }
> > !
> > ! EndTransactionBlock(ignore);
> > ! }
> > break;
> >
> > case TRANS_STMT_ROLLBACK:
> > diff -Ncr --exclude-from=diff-ignore 10bgwriter/src/include/access/xact.h 13commitOpt/src/include/access/xact.h
> > *** 10bgwriter/src/include/access/xact.h 2004-06-08 17:48:36.000000000 -0400
> > --- 13commitOpt/src/include/access/xact.h 2004-06-09 12:00:19.000000000 -0400
> > ***************
> > *** 171,177 ****
> > extern void CommitTransactionCommand(void);
> > extern void AbortCurrentTransaction(void);
> > extern void BeginTransactionBlock(void);
> > ! extern void EndTransactionBlock(void);
> > extern bool IsSubTransaction(void);
> > extern bool IsTransactionBlock(void);
> > extern bool IsTransactionOrTransactionBlock(void);
> > --- 171,177 ----
> > extern void CommitTransactionCommand(void);
> > extern void AbortCurrentTransaction(void);
> > extern void BeginTransactionBlock(void);
> > ! extern void EndTransactionBlock(bool ignore);
> > extern bool IsSubTransaction(void);
> > extern bool IsTransactionBlock(void);
> > extern bool IsTransactionOrTransactionBlock(void);
> >
> >
> > ------------------------------------------------------------------------
> >
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 7: don't forget to increase your free space map settings
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faqs/FAQ.html
>
-- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> We are considering allowing COMMIT IGNORE ABORT for scripts that want to
> do a subtransaction, but don't care if it fails, and because it is a
> script, they can't test the return value to send ROLLBACK:
While we clearly want this functionality, I tend to agree with Barry
that COMMIT IGNORE ABORT (and the other variants that have been floated)
is a horrid, confusing name for it. I would suggest using END with some
modifier, instead. Perhaps
END [ WORK | TRANSACTION ] [ IGNORE ERRORS ]
END doesn't so directly imply that you are trying to commit a failed
transaction.
regards, tom lane
On Thu, Jun 10, 2004 at 03:39:14PM -0400, Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > We are considering allowing COMMIT IGNORE ABORT for scripts that want to > > do a subtransaction, but don't care if it fails, and because it is a > > script, they can't test the return value to send ROLLBACK: > > While we clearly want this functionality, I tend to agree with Barry > that COMMIT IGNORE ABORT (and the other variants that have been floated) > is a horrid, confusing name for it. I would suggest using END with some > modifier, instead. Perhaps > > END [ WORK | TRANSACTION ] [ IGNORE ERRORS ] > > END doesn't so directly imply that you are trying to commit a failed > transaction. The problem with END is how about executing it inside a PL/pgSQL function. Can we distinguish it from plpgsql's END? Also, COMMITing an aborted main transaction is the same as ENDing it; and in fact, it's the same as ROLLBACK. Why is it more confusing for a subtransaction to behave the same? I agree that the grammar I proposed is wrong. I guess I can ask for two words and then strcmp() them to "ignore errors"? -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "La naturaleza, tan frágil, tan expuesta a la muerte... y tan viva"
Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> On Thu, Jun 10, 2004 at 03:39:14PM -0400, Tom Lane wrote:
>> END doesn't so directly imply that you are trying to commit a failed
>> transaction.
> The problem with END is how about executing it inside a PL/pgSQL
> function. Can we distinguish it from plpgsql's END?
We're going to have to deal with that on the BEGIN side anyway.
A reasonable possibility would be to require the TRANSACTION word
to appear when you do it in plpgsql.
> Also, COMMITing an aborted main transaction is the same as ENDing it;
> and in fact, it's the same as ROLLBACK. Why is it more confusing for a
> subtransaction to behave the same?
But the point here is that the behavior would *not* be the same.
regards, tom lane
Tom Lane wisely wrote: > While we clearly want this functionality, I tend to agree with Barry > that COMMIT IGNORE ABORT (and the other variants that have > been floated) > is a horrid, confusing name for it. I would suggest using > END with some > modifier, instead. Perhaps > > END [ WORK | TRANSACTION ] [ IGNORE ERRORS ] > > END doesn't so directly imply that you are trying to commit a failed > transaction. > Would it make more sense to specify at the time the optional subtransaction is committed that it is not critical to the completion of the outer transaction? BEGIN; BEGIN; DROP TABLE foo; COMMIT NON_CRITICAL; CREATE TABLE foo (i int); COMMIT; I don't 'get' the nested transaction code, so I don't know how horrible this would be to write. It just seemed more useful, because you could specify which sub-transactions are show stoppers, and which ones aren't. Or if I'm completely off base, please forgive my intrusion. Paul
On Wed, Jun 09, 2004 at 11:32:08PM -0700, Stephan Szabo wrote: > > Unfortunately, I've gotten it to fail, but I haven't looked in depth (I'm > > at work, so I'm doing it during compilations and such.) [...] > Okay - I think I see what's going on here. > > It looks like deferredTriggerInvokeEvents is being run (immediate_only), > but since deferredTriggers->events_imm is NULL it's using > deferredTriggers->events as the start of the list to check, but this value > isn't getting reset in DeferredTriggerEndSubXact in the case that the > entire list was created in an aborted subtransaction. Ok, thanks for the test and diagnostics; patch attached. I'll see if I can find other situations like this. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "No hay cielo posible sin hundir nuestras raíces en la profundidad de la tierra" (Malucha Pinto)
Attachment
"Bort, Paul" <pbort@tmwsystems.com> writes: > Would it make more sense to specify at the time the optional subtransaction > is committed that it is not critical to the completion of the outer > transaction? > > BEGIN; > BEGIN; > DROP TABLE foo; > COMMIT NON_CRITICAL; > CREATE TABLE foo (i int); > COMMIT; I assumed that was what was being proposed. It doesn't make sense to have a single flag on the entire outer transaction since there could have been multiple inner transactions, not all of which are unimportant. Hm, perhaps a parallel to "CREATE OR REPLACE" would be "COMMIT OR ROLLBACK". I'm not sure if I'm serious about that or joking though. -- greg
On Thu, 10 Jun 2004, Alvaro Herrera wrote:
> On Wed, Jun 09, 2004 at 11:32:08PM -0700, Stephan Szabo wrote:
>
> > > Unfortunately, I've gotten it to fail, but I haven't looked in depth (I'm
> > > at work, so I'm doing it during compilations and such.)
>
> [...]
>
> > Okay - I think I see what's going on here.
> >
> > It looks like deferredTriggerInvokeEvents is being run (immediate_only),
> > but since deferredTriggers->events_imm is NULL it's using
> > deferredTriggers->events as the start of the list to check, but this value
> > isn't getting reset in DeferredTriggerEndSubXact in the case that the
> > entire list was created in an aborted subtransaction.
>
> Ok, thanks for the test and diagnostics; patch attached. I'll see if I
> can find other situations like this.
As a question, what was the general assumption about what the following
should do (using a modification of the original test case)?
DROP TABLE foo CASCADE;
DROP TABLE bar CASCADE;
CREATE TABLE foo (A INT UNIQUE);
CREATE TABLE bar (A INT REFERENCES foo(A) DEFERRABLE);
DELETE FROM bar;
DELETE FROM foo;
INSERT INTO foo VALUES (1);
INSERT INTO foo VALUES (2);
BEGIN; SET CONSTRAINTS ALL DEFERRED; INSERT INTO bar VALUES (1); BEGIN; INSERT INTO bar
VALUES(3); COMMIT; BEGIN; BEGIN; INSERT INTO bar VALUES (4);
COMMIT; INSERT INTO foo VALUES (3); SET CONSTRAINTS ALL IMMEDIATE; ROLLBACK;
-- move this set constraints down below
-- SET CONSTRAINTS ALL DEFERRED; BEGIN; INSERT INTO bar VALUES (5); --(1) COMMIT;
SET CONSTRAINTS ALL DEFERRED; BEGIN; BEGIN; INSERT INTO bar VALUES (6);
ROLLBACK; COMMIT; BEGIN; INSERT INTO bar VALUES (7); COMMIT; BEGIN;
BEGIN; INSERT INTO bar VALUES (9); COMMIT; COMMIT; INSERT INTO
fooVALUES(3); INSERT INTO foo VALUES(5); INSERT INTO foo VALUES(7); INSERT INTO foo VALUES(9);
COMMIT;
Should the statement at (1) fail because the constraint is not immediately
satisfied, or should it pass because the set constraints all immediate was
part of a subtransaction that didn't commit?
On Sat, 12 Jun 2004, Alvaro Herrera wrote: > On Sat, Jun 12, 2004 at 10:27:15AM -0700, Stephan Szabo wrote: > > > As a question, what was the general assumption about what the following > > should do (using a modification of the original test case)? > > [...] > > > Should the statement at (1) fail because the constraint is not immediately > > satisfied, or should it pass because the set constraints all immediate was > > part of a subtransaction that didn't commit? > > I think the correct answer should be that (1) should pass because the > all immediate set would be rolled back. However, GUC vars do not > presently roll back so it doesn't work. In fact, if GUC vars do roll > back, then there is no need to explicitly set all constraints to > deferred, because it would roll back to that value automatically. > > GUC vars are the remaining item for correct nested xacts AFAICS. AFAIK, SET CONSTRAINTS isn't modifying a GUC variable, it gets turned into a command that goes to DeferredTriggerSetState in trigger.c.