It looks good now. Thank you again.
2019年9月9日(月) 17:43 Peter Eisentraut :
> On 2019-09-09 05:58, fn ln wrote:
> > Confirmed. Thank you all for your help.
> >
> > The only concern is that this test:
> >
> >SET TRANSACTION READ WRITE\; COMMIT AND CHAIN; -- error
> >SHOW transaction_read_on
On 2019-09-09 05:58, fn ln wrote:
> Confirmed. Thank you all for your help.
>
> The only concern is that this test:
>
> SET TRANSACTION READ WRITE\; COMMIT AND CHAIN; -- error
> SHOW transaction_read_only;
>
> SET TRANSACTION READ WRITE\; ROLLBACK AND CHAIN; -- error
> SHOW transac
Confirmed. Thank you all for your help.
The only concern is that this test:
SET TRANSACTION READ WRITE\; COMMIT AND CHAIN; -- error
SHOW transaction_read_only;
SET TRANSACTION READ WRITE\; ROLLBACK AND CHAIN; -- error
SHOW transaction_read_only;
makes more sense with READ ONLY bec
On 2019-09-07 18:32, fn ln wrote:
>> Missed them somehow. But I don't think they're quite sufficient. I think
>> at least we also need tests that test things like multi-statement
>> exec_simple-query() *with* explicit transactions and chaining.
> Added a few more tests for that.
I committed this p
No, but instead always do an implicit COMMIT after each statement, like
SELECT 1; SELECT 2; (not \;) in psql.
The PostgreSQL document even states that 'Issuing COMMIT when not inside a
transaction does no harm, but it will provoke a warning message.' for a
long time,
but in fact it have side-effect
Now, I'd prefer error in all cases, no doubt about that, which might be
considered a regression. A way around that could be to have a GUC decide
between a strict behavior (error) and the old behavior (warning).
I think it's more better to have a GUC to disable implicit transaction
'block' feat
> Missed them somehow. But I don't think they're quite sufficient. I think
> at least we also need tests that test things like multi-statement
> exec_simple-query() *with* explicit transactions and chaining.
Added a few more tests for that.
> Now, I'd prefer error in all cases, no doubt about that
I made another patch for that.
I don't have much confidence with my English spelling so further
improvements may be needed.
If it is too much a change and potential regression, then I think that the
"and chain" variants should be consistent and just raise warnings.
We don't have an exact a
Hi,
On 2019-09-06 16:54:15 +0900, Michael Paquier wrote:
> On Thu, Sep 05, 2019 at 02:11:35PM -0700, Andres Freund wrote:
> > On 2019-09-05 14:16:11 +0200, Peter Eisentraut wrote:
> >> I'm content with this patch.
> >
> > Would need tests.
>
> The latest patch sends adds coverage for all the new
> Looking at the latest patch, the comment blocks on top of TBLOCK_STARTED
> and TBLOCK_IMPLICIT_INPROGRESS in EndTransactionBlock() need an update
> to mention the difference of behavior with chained transactions. And
> the same comment rework should be done in UserAbortTransactionB
Hello,
I do think the fact that COMMIT in multi-statement implicit transaction
has some usecase, is an argument for just implementing it properly...
Like Peter, I would also keep an ERROR for now, as we could always
relax that later on.
I can agree with both warning and error, but for me t
Peter, I would also keep an ERROR for now, as we could always
relax that later on.
Looking at the latest patch, the comment blocks on top of TBLOCK_STARTED
and TBLOCK_IMPLICIT_INPROGRESS in EndTransactionBlock() need an update
to mention the difference of behavior with chained transactions. And
Hi,
On 2019-09-05 14:16:11 +0200, Peter Eisentraut wrote:
> On 2019-09-04 16:49, fn ln wrote:
> > I made another patch for suggested behavior (COMMIT/ROLLBACK AND CHAIN
> > now gives us an error when used in an implicit block).
>
> I'm content with this patch.
Would need tests.
> Better disab
On 2019-09-04 16:49, fn ln wrote:
> I made another patch for suggested behavior (COMMIT/ROLLBACK AND CHAIN
> now gives us an error when used in an implicit block).
I'm content with this patch. Better disable questionable cases now and
maybe re-enable them later if someone wants to make a case for
I made another patch for suggested behavior (COMMIT/ROLLBACK AND CHAIN now
gives us an error when used in an implicit block).
2019年9月4日(水) 16:53 Andres Freund :
> Hi,
>
> On 2019-09-03 11:54:57 +0200, Peter Eisentraut wrote:
> > On 2019-08-29 16:58, Fabien COELHO wrote:
> > >
> > >> Thanks, I got
Hi,
On 2019-09-03 11:54:57 +0200, Peter Eisentraut wrote:
> On 2019-08-29 16:58, Fabien COELHO wrote:
> >
> >> Thanks, I got it. I have never made a patch before so I'll keep it in my
> >> mind. Self-contained patch is now attached.
> >
> > v3 applies, compiles, "make check" ok.
> >
> > I turn
We have three options:
1. Prohibit AND CHAIN outside a transaction block, but do nothing in
plain COMMIT/ROLLBACK or AND NO CHAIN.
2. Deal "there is no transaction in progress" (and "there is already a
transaction in progress" if needed) as an error.
3. Leave as it is.
Option 1 makes overal
v3 applies, compiles, "make check" ok.
I turned it ready on the app.
Should we make it an error instead of a warning?
ISTM that it made sense to have the same behavior as out of transaction
COMMIT or ROLLBACK.
--
Fabien.
On 2019-08-29 16:58, Fabien COELHO wrote:
>
>> Thanks, I got it. I have never made a patch before so I'll keep it in my
>> mind. Self-contained patch is now attached.
>
> v3 applies, compiles, "make check" ok.
>
> I turned it ready on the app.
Should we make it an error instead of a warning?
> The usual approach is to send self-contained and numbered patches,
> eg "chain-fix-1.patch", "chain-fix-2.patch", and so on, unless there are
> complex patches designed to be committed in stages.
Thanks, I got it. I have never made a patch before so I'll keep it in my
mind.
Self-contained patch
Thanks, I got it. I have never made a patch before so I'll keep it in my
mind. Self-contained patch is now attached.
v3 applies, compiles, "make check" ok.
I turned it ready on the app.
--
Fabien
Hello,
transaction_read_only must be 'on' because AND CHAIN test sets the
default_transaction_read_only to 'on'.
Failure of this test means that the transaction was chained from an
implicit transaction, which is not our desired behavior. Perhaps you are
using a wrong binary?
Nope, I blind
Added two kinds of test for the implicit transaction: in single query and
in implicit block.
The patch file is now created with Unix-style line ending (LF).
2019年8月29日(木) 15:30 Fabien COELHO :
>
> Hello,
>
> > COMMIT AND CHAIN in implicit block leaves blockState as TBLOCK_STARTED,
> > which doesn
transaction_read_only must be 'on' because AND CHAIN test sets the
default_transaction_read_only to 'on'.
Failure of this test means that the transaction was chained from an
implicit transaction, which is not our desired behavior.
Perhaps you are using a wrong binary?
2019年8月29日(木) 21:10 Fabien C
Hello,
Added two kinds of test for the implicit transaction: in single query and
in implicit block.
Ok.
The patch file is now created with Unix-style line ending (LF).
Thanks.
Patch applies and compiles cleanly. However, "make check" is not ok
on the added tests.
SHOW transaction_re
COMMIT AND CHAIN in implicit block leaves blockState as TBLOCK_STARTED,
which doesn't trigger the chaining.
but ROLLBACK AND CHAIN sets the blockState into TBLOCK_ABORT_PENDING, so
the chaining is triggered.
I think disabling s->chain beforehand should do the desired behavior.
2019年8月25日(日) 18:1
Patch works for me, and solution seems appropriate. It should be committed
for pg 12.0.
I have listed this as an open issue of the upcoming pg 12:
https://wiki.postgresql.org/wiki/PostgreSQL_12_Open_Items#Open_Issues
--
Fabien.
Hello,
COMMIT AND CHAIN in implicit block leaves blockState as TBLOCK_STARTED,
which doesn't trigger the chaining. but ROLLBACK AND CHAIN sets the
blockState into TBLOCK_ABORT_PENDING, so the chaining is triggered.
I think disabling s->chain beforehand should do the desired behavior.
Patc
The following bug has been logged on the website:
Bug reference: 15977
Logged by: mtlh kdvt
Email address: emuser20140...@gmail.com
PostgreSQL version: 12beta3
Operating system: Windows
Description:
When a ROLLBACK AND CHAIN command is executed in the implicit transaction
Hallo Peter,
In "xact.c", maybe I'd assign blockState in the else branch, instead of
overriding it?
I think it was better the way it is, since logically the block state is
first set, then set again after the new transaction starts.
Ok.
About the static _SPI_{commit,rollback} functions: I
Patch has been committed, thanks.
On 2019-03-18 21:20, Fabien COELHO wrote:
> Minor remarks:
>
> In "xact.c", maybe I'd assign blockState in the else branch, instead of
> overriding it?
I think it was better the way it is, since logically the block state is
first set, then set again after the n
Hallo Peter,
Updated patch. I have squashed the two previously separate patches
together in this one.
Ok.
I do not understand the value of the SAVEPOINT in the tests.
The purpose of the SAVEPOINT in the test is because it exercises
different switch cases in CommitTransactionCommand() an
On 2019-02-16 06:22, Andres Freund wrote:
>> +static int save_XactIsoLevel;
>> +static bool save_XactReadOnly;
>> +static bool save_XactDeferrable;
>
> We normally don't define variables in the middle of a file? Also, why
> do these need to be global vars rather than defined where we do
> chaini
6b23163929 100644
--- a/src/backend/catalog/sql_features.txt
+++ b/src/backend/catalog/sql_features.txt
@@ -443,7 +443,7 @@ T213INSTEAD OF triggers YES
T231 Sensitive cursors YES
T241 START TRANSACTION statement YES
Hi,
On 2019-01-02 16:02:21 +0100, Peter Eisentraut wrote:
> +++ b/src/backend/access/transam/xact.c
> @@ -189,6 +189,7 @@ typedef struct TransactionStateData
> boolstartedInRecovery; /* did we start in recovery? */
> booldidLogXid; /* has xid b
Hello Peter,
Sure. Within a read-only tx, it could check that transaction_read_only is
on, and still on when chained, though.
I think the tests prove the point that the values are set and unset and
reset in various scenarios. We don't need to test every single
combination, that's not the jo
Hello Peter,
I'm wary of changing the SPI_commit and SPI_rollback interfaces which are
certainly being used outside the source tree and could break countless
code, and it seems quite unclean that commit and rollback would do
anything else but committing or rollbacking.
These are new as of PG
On 26/12/2018 09:47, Fabien COELHO wrote:
> I'm wary of changing the SPI_commit and SPI_rollback interfaces which are
> certainly being used outside the source tree and could break countless
> code, and it seems quite unclean that commit and rollback would do
> anything else but committing or ro
log/sql_features.txt
@@ -443,7 +443,7 @@ T213INSTEAD OF triggers YES
T231 Sensitive cursors YES
T241 START TRANSACTION statement YES
T251 SET TRANSACTION statement: LOCAL option NO
-T2
On 2018-Dec-26, Fabien COELHO wrote:
> > > 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
Updated patch attached. The previous (v2) patch apparently didn't apply
anymore.
Second patch applies cleanly, compiles, "make check" ok.
Also about testing, I'd do less rounds, 4 quite seems enough.
--
Fabien.
Updated patch attached. The previous (v2) patch apparently didn't apply
anymore.
Second patch applies cleanly, compiles, "make check" ok.
As I do not know much about the SPI stuff, some of the comments below may
be very stupid.
I'm wary of changing the SPI_commit and SPI_rollback interfa
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
riggers YES
T231 Sensitive cursors YES
T241 START TRANSACTION statement YES
T251 SET TRANSACTION statement: LOCAL option NO
-T261 Chained transactionsNO
+T261 Chained transac
Hello Peter,
Attached is a rebased patch set. No functionality changes.
Patch applies cleanly, compile, global make check ok, doc gen ok.
Shouldn't psql tab completion be updated as well?
About the code:
I must admit that do not like much the three global variables &
Save/Restore functio
On 05/04/2018 10:35, Heikki Linnakangas wrote:
> On 15/03/18 18:39, Alvaro Herrera wrote:
>>> From 517bc6d9fefdee9135857d9562f644f2984ace32 Mon Sep 17 00:00:00 2001
>>> From: Peter Eisentraut
>>> Date: Sun, 18 Feb 2018 09:33:53 -0500
>>> Subject: [PATCH v1 6/8] Turn transaction_isolation into GUC
+ b/src/backend/catalog/sql_features.txt
@@ -443,7 +443,7 @@ T213INSTEAD OF triggers YES
T231 Sensitive cursors YES
T241 START TRANSACTION statement YES
T251 SET TRANSACTION statement: LOCAL option
On Tue, Jul 17, 2018 at 08:21:20PM +0300, Heikki Linnakangas wrote:
> Oh, is that all it does? That's disappointing, because that's a lot less
> powerful than how I understand chained transactions. And at the same time
> relieving, because that's a lot simpler to imp
On 01/03/18 05:35, Peter Eisentraut wrote:
The SQL standard offers the "chained transactions" feature to address
this. The new command variants COMMIT AND CHAIN and ROLLBACK AND CHAIN
immediately start a new transaction with the characteristics (isolation
level, read/write, deferrab
On 4/5/18 04:35, Heikki Linnakangas wrote:
> With this patch, this stops working:
>
> set transaction_isolation='default';
But why is this supposed to work? This form is not documented anywhere.
You can use RESET or SET TO DEFAULT.
I suspect this is some artifact in the implementation that thi
On 15/03/18 18:39, Alvaro Herrera wrote:
From 517bc6d9fefdee9135857d9562f644f2984ace32 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Sun, 18 Feb 2018 09:33:53 -0500
Subject: [PATCH v1 6/8] Turn transaction_isolation into GUC enum
XXX no idea why it was done the way it was, but this seem
On 3/15/18 12:39, Alvaro Herrera wrote:
>> Subject: [PATCH v1 1/8] Update function comments
>> Subject: [PATCH v1 2/8] Rename TransactionChain functions
>> Subject: [PATCH v1 3/8] Simplify parse representation of savepoint commands
>> Subject: [PATCH v1 4/8] Improve savepoint error messages
>> Subj
Peter Eisentraut wrote:
> Subject: [PATCH v1 1/8] Update function comments
>
> After a6542a4b6870a019cd952d055d2e7af2da2fe102, some function comments
> were misplaced. Fix that.
Note typo WarnNoTranactionChain in one comment. The patch leaves
CheckTransactionChain with no comment whatsoever; m
On 2018-03-05 09:21:33 +, Simon Riggs wrote:
> On 2 March 2018 at 07:18, Andres Freund wrote:
> > Hi,
> >
> > On 2018-02-28 22:35:52 -0500, Peter Eisentraut wrote:
> >> This feature is meant to help manage transaction isolation in
> >> procedures.
> >
> > This is a major new feature, submitted
On 2 March 2018 at 07:18, Andres Freund wrote:
> Hi,
>
> On 2018-02-28 22:35:52 -0500, Peter Eisentraut wrote:
>> This feature is meant to help manage transaction isolation in
>> procedures.
>
> This is a major new feature, submitted the evening before the last CF
> for v11 starts. Therefore I thi
Hi,
On 2018-02-28 22:35:52 -0500, Peter Eisentraut wrote:
> This feature is meant to help manage transaction isolation in
> procedures.
This is a major new feature, submitted the evening before the last CF
for v11 starts. Therefore I think it should just be moved to the next
fest, it doesn't seem
which would create code that is difficult to manage.
The SQL standard offers the "chained transactions" feature to address
this. The new command variants COMMIT AND CHAIN and ROLLBACK AND CHAIN
immediately start a new transaction with the characteristics (isolation
level, read/write, def
57 matches
Mail list logo
