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;
        COMMIT
        CREATE 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
  [EMAIL PROTECTED]               |  (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 8: explain analyze is your friend

Reply via email to