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
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
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
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
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
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'
> 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
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
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
> 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 UserAbortTransactionBlock()
> for
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
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 code paths
added. Do you have something else in mind?
>> Better
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
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
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
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
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
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
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
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
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
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日(日)
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.
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
29 matches
Mail list logo
