Re: chained transactions - Mailing list pgsql-hackers
From | Fabien COELHO |
---|---|
Subject | Re: chained transactions |
Date | |
Msg-id | alpine.DEB.2.21.1812260824270.32444@lancre Whole thread Raw |
In response to | Re: chained transactions (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>) |
Responses |
Re: chained transactions
Re: chained transactions |
List | pgsql-hackers |
Hello Peter, >> Shouldn't psql tab completion be updated as well? > > Done. (I only did the AND CHAIN, not the AND NO CHAIN.) Sure, that is the useful part. >> I must admit that do not like much the three global variables & >> Save/Restore functions. I'd suggest saving directly into 3 local variables >> in function CommitTransactionCommand, and restoring them when needed. Code >> should not be longer. I'd would not bother to skip saving when not >> chaining. > > We're also using those functions in the 0002 patch. Hmmm. I have not looked at the second one yet. > Should we just repeat the code three times, or use macros? I try to avoid global variables when possible as a principle, because I paid to learn that they are bad:-) Maybe I'd do a local struct, declare an instance within the function, and write two functions to dump/restore the transaction status variables into the referenced struct. Maybe this is not worth the effort. >> Copying & comparing nodes are updated. Should making, outing and reading >> nodes also be updated? > > TransactionStmt isn't covered by the node serialization functions, so I > didn't see anything to update. What did you have in mind? Sigh. I had in mind that the serialization feature would work with all possible nodes, not just some of them… which seems quite naïve. The whole make/copy/cmp/in/out functions depress me, all this verbose code should be automatically generated from struct declarations. I'm pretty sure there are hidden bugs in there. >> About the tests: I'd suggest to use more options on the different tests, >> eg SERIALIZABLE, READ ONLY… Also ISTM that tests should show >> transaction_read_only value as well. > > OK, updated a bit. (Using READ ONLY doesn't work because then you can't > write anything inside the transaction.) Sure. Within a read-only tx, it could check that transaction_read_only is on, and still on when chained, though. > Updated patch attached. First patch applies cleanly, compiles, make check ok. Further remarks, distinct from the above comments and suggestions: The documentation should probably tell in the compatibility sections that AND CHAIN is standard conforming. In the automatic rollback tests, maybe I'd insert 3 just once, and use another value for the chained transaction. Tests may eventually vary BEGIN/START, COMMIT/END, ROLLBACK/ABORT. As you added a SAVEPOINT, maybe I'd try rollbacking to it. -- Fabien.
pgsql-hackers by date: