Re: php with postgres - Mailing list pgsql-hackers

Marcus B?rger wrote:
> BM> I believe this should be BEGIN;ROLLBACK;RESET ALL; because our default
> BM> for a client that disconnects is to abort the transaction.
> 
> >>  - If protocol version >= 3 and transaction status == PQTRANS_IDLE:
> >>      "RESET ALL;"
> >>  - If protocol version >= 3 and transaction status != PQTRANS_IDLE:
> >>      "COMMIT;RESET ALL;"
> 
> BM> Should be "ROLLBACK;RESET ALL;".
> 
> Because of above mentioned default?
> The problem i have is that now we do the COMMIT - so this behavior change can
> only go into php 5 with a big notice.

Well, our behavior has always been to abort any open transctions on
client exit.  In fact, it is so obvious, we don't even document it.

There are some SQL databases that commit on client exit, but that sounds
just wrong to us.  (They might distinguish between client close and
client connection failure, but I am just guessing.)  You could have
removed money from one account but not added it to a second account when
your client died.

You can try it yourself by starting a transaction in psql, inserting
some data, and exiting psql.  When you reconnect, your data will not be
in the database.

Pooled connections should work just like non-pooled connections --- in
fact, that is what we are trying to improve with RESET ALL, so it seems
like a clear bug fix to change the code from COMMIT to ROLLBACK.

Of course, you know the PHP community better than I do.

> BM> Nice version test code, sounds good.
> 
> >> 
> >> Disconnect:
> >>  - When PQprotocolVersion() And PQtransactionStatus() are available then
> >>    i check whether status is PQTRANS_IDLE. If so i do:
> >>      "ROLLBACK;"
> >>  - If the functions are not available in the client libs i do:
> >>      "BEGIN;ROLLBACK;"
> >> 
> >> Does this sound the correct behavior?
> 
> BM> I am confused why you are doing stuff on connect and disconnect.  Seems
> BM> it should all be done on disconnect so you don't leave open transactions
> BM> in the pooled connections --- it will keep locks around too long and
> BM> reduce the usefulness of vacuum.  If we clean up everything on
> BM> disconnect, aren't we sure that the connection status will be fine?
> 
> >> And would "select split_part(version(), ' ', 2);" be too much of a slowdown to
> >> do the version detection in the startup sequence completely correct?
> 
> BM> Seems fine.  Since you are doing pooled connections, you shouldn't be
> BM> doing this too often anyway.
> 
> I believe the point was "RESET ALL;". But maybe i can move all but that into
> shutdown. I mean for me that sounds good. Only someone must enlighten me if
> there could be a problem with that reset.

No, no problems.  You are guaranteed to get a reset values when you
first start a backend, so there isn't anything to do on startup if you
get the disconnect cleaned up properly, and as I mentioned, there are
advantages to having that stuff done in shutdown so the transaction
isn't left open.

I see this pooled connection stuff is more complicated that it first
appears.

--  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