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

Reply via email to