On Sunday 30 April 2006 23:05, Chris Darroch wrote:
> > OK, I started working on this (skeleton of of the patch is ready), but
> > before I go any further, could you give me a hint on the semantics of
> > the above. What I'm not getting is APR_DBD_TRANS_COMMIT v.
> > APR_DBD_TRANS_COMMIT_ON_SUCCESS. Isn't that the same? I mean, you can't
> > forcefully commit anything - the database won't take it.
There's rather more than that: after any failed operation within
a transaction, apr_dbd assumes transaction failure and declines
to attempt any further ops, without reference to the backend.
Thus:
> > Or is there something I missed here...
>
> Well, this may not make sense, but what I was thinking was
> that most drivers currently seem to decide whether to rollback
> or commit based on whether some errors have occurred in other
> calls and an internal flag has been set. For example, from pgsql:
>
> if (trans->errnum) {
> trans->errnum = 0;
> res = PQexec(trans->handle->conn, "ROLLBACK");
> }
> else {
> res = PQexec(trans->handle->conn, "COMMIT");
> }
Indeedie.
> What I was thinking was that this is the COMMIT_ON_SUCCESS
> default logic. ROLLBACK obviously forces a rollback. COMMIT
> would attempt a DB commit regardless of whether the driver
> had flagged anything in previously calls. So, like this:
Bearing in mind the above, under what circumstances would
that be meaningful? It's predicated on an assumption that
we have recoverable errors within a transaction. But apr_dbd
doesn't support that (except insofar as a driver may correct
errors within its black box). Or am I missing something?
> switch (trans->mode) {
> case APR_DBD_TRANSACTION_COMMIT:
> res = PQexec(trans->handle->conn, "COMMIT");
> break;
The only usage case I can see where that would make sense is
immediately after an error return, to perform some error-correction
then commit. But any such error-correction would necessarily
take you outside the apr_dbd layer and into direct ops on the
native handle. So having apr_dbd support the commit seems
to have little point, AFAICS.
> case APR_DBD_TRANSACTION_COMMIT_ON_SUCCESS:
> if (trans->errnum) {
> trans->errnum = 0;
> res = PQexec(trans->handle->conn, "ROLLBACK");
> } else
> res = PQexec(trans->handle->conn, "COMMIT");
> break;
> case APR_DBD_TRANSACTION_ROLLBACK:
> res = PQexec(trans->handle->conn, "ROLLBACK");
> break;
> }
That's fine.
> That's just a quick sketch ... if that's dumb, by all means
> do something else! I should be able to focus on this a bit
> next week, as I said, if that helps any. Cheers,
I did express a preference before, but it's not a strong one.
I would also accept Bojan's original patch (simply add
apr_dbd_transaction_rollback to the API), or your scheme
outlined here if you can convince me force-commit has a role.
--
Nick Kew
