Thread: [BUGS] Two phase commit in ECPG
Hi all, I found that ecpg has not been supporting COMMIT PREPARED and ROLLBACK PREPARED since version 9.0.2. For example, if the test code does, EXEC SQL BEGIN; EXEC SQL INSERT INTO t1 VALUES(1); EXEC SQL PREPARE TRANSACTION 'gxid'; EXEC SQL COMMIT PREPARED 'gxid'; I got error "COMMIT PREPARED cannot run inside a transaction block". This cause is that the "begin transaction" is issued automatically before executing COMMIT PREPARED if we're not in auto commit. The Commit 816b008eaf1a1ff1069f3bafff363a9a8bf04a21 fixed bug of incorrect status calculation but at the same time it became the cause of this behavior. Is this a bug? Attached 001 patch fixes this issue and add regression test for two phase commit in ecpg. Also, in spite of ecpg identifies these two commands as transaction command similar to other transaction commands like commit and rollback the documentation doesn't mentioned about these at all. Attached 002 patch add description about these to documentation. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Attachment
Dear Sawada-san, > This cause is that the "begin transaction" is issued automatically > before executing COMMIT PREPARED if we're not in auto commit. The > Commit 816b008eaf1a1ff1069f3bafff363a9a8bf04a21 fixed bug of > incorrect > status calculation but at the same time it became the cause of this > behavior. > Is this a bug? I'd say so, yes. As soon as I find time I'll get to it including back porting your patch. Thank you very much for spotting and fixing. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
On Sat, Mar 4, 2017 at 4:11 AM, Michael Meskes <meskes@postgresql.org> wrote: > Dear Sawada-san, > >> This cause is that the "begin transaction" is issued automatically >> before executing COMMIT PREPARED if we're not in auto commit. The >> Commit 816b008eaf1a1ff1069f3bafff363a9a8bf04a21 fixed bug of >> incorrect >> status calculation but at the same time it became the cause of this >> behavior. >> Is this a bug? > > I'd say so, yes. > > As soon as I find time I'll get to it including back porting your > patch. Thanks Previous 002 patch lacked to add describing PREPARE TRANSACTION. Attached updated 002 patch. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Attachment
> Previous 002 patch lacked to add describing PREPARE TRANSACTION. > Attached updated 002 patch. I just committed both patches and a backport of the bug fix itself. Thanks again for finding and fixing. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
On Tue, Mar 14, 2017 at 1:35 AM, Michael Meskes <meskes@postgresql.org> wrote: >> Previous 002 patch lacked to add describing PREPARE TRANSACTION. >> Attached updated 002 patch. > > I just committed both patches and a backport of the bug fix itself. > Thanks again for finding and fixing. Regression tests for sql/twophase is failing while performing the test with make installcheck. + [NO_PID]: ecpg_check_PQresult on line 32: bad response - ERROR: prepared transactions are disabled + HINT: Set max_prepared_transactions to a nonzero value. Setting max_prepared_transactions accordingly fixes the issue. But, I'm not sure whether this test should be performed by installcheck by default or should only be performed by make installcheck-prepared-txns. -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com
On Fri, Mar 17, 2017 at 12:17 PM, Kuntal Ghosh <kuntalghosh.2007@gmail.com> wrote: > On Tue, Mar 14, 2017 at 1:35 AM, Michael Meskes <meskes@postgresql.org> wrote: >>> Previous 002 patch lacked to add describing PREPARE TRANSACTION. >>> Attached updated 002 patch. >> >> I just committed both patches and a backport of the bug fix itself. >> Thanks again for finding and fixing. > Regression tests for sql/twophase is failing while performing the test > with make installcheck. > + [NO_PID]: ecpg_check_PQresult on line 32: bad response - ERROR: > prepared transactions are disabled > + HINT: Set max_prepared_transactions to a nonzero value. > > Setting max_prepared_transactions accordingly fixes the issue. But, > I'm not sure whether this test should be performed by installcheck by > default or should only be performed by make > installcheck-prepared-txns. > Thank you for pointing out. Yeah, I agree that the twophase regression test should not be performed by default as long as the default value of max_prepared_transactions is 0. Similar to this, the isolation check regression test does same thing. Attached patch removes sql/twophase from schedule file and add new type of regression test. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Attachment
On Fri, Mar 17, 2017 at 4:34 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote: > On Fri, Mar 17, 2017 at 12:17 PM, Kuntal Ghosh > <kuntalghosh.2007@gmail.com> wrote: >> On Tue, Mar 14, 2017 at 1:35 AM, Michael Meskes <meskes@postgresql.org> wrote: >>>> Previous 002 patch lacked to add describing PREPARE TRANSACTION. >>>> Attached updated 002 patch. >>> >>> I just committed both patches and a backport of the bug fix itself. >>> Thanks again for finding and fixing. >> Regression tests for sql/twophase is failing while performing the test >> with make installcheck. >> + [NO_PID]: ecpg_check_PQresult on line 32: bad response - ERROR: >> prepared transactions are disabled >> + HINT: Set max_prepared_transactions to a nonzero value. >> >> Setting max_prepared_transactions accordingly fixes the issue. But, >> I'm not sure whether this test should be performed by installcheck by >> default or should only be performed by make >> installcheck-prepared-txns. >> > > Thank you for pointing out. > Yeah, I agree that the twophase regression test should not be > performed by default as long as the default value of > max_prepared_transactions is 0. Similar to this, the isolation check > regression test does same thing. Attached patch removes sql/twophase > from schedule file and add new type of regression test. > The patch looks good. I've performed installcheck and installcheck-prepared-txns. It's working as it should be. -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com
> Thank you for pointing out. > Yeah, I agree that the twophase regression test should not be > performed by default as long as the default value of > max_prepared_transactions is 0. Similar to this, the isolation check > regression test does same thing. Attached patch removes sql/twophase > from schedule file and add new type of regression test. Would it be possible to include it in "make check"? Any check that needs manual interaction will not be executed nearly is often as the others and become much less useful imo. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
On Fri, Mar 17, 2017 at 5:50 PM, Michael Meskes <meskes@postgresql.org> wrote: >> Thank you for pointing out. >> Yeah, I agree that the twophase regression test should not be >> performed by default as long as the default value of >> max_prepared_transactions is 0. Similar to this, the isolation check >> regression test does same thing. Attached patch removes sql/twophase >> from schedule file and add new type of regression test. > > Would it be possible to include it in "make check"? Any check that > needs manual interaction will not be executed nearly is often as the > others and become much less useful imo. > Yes. I added two-phase commit test to "make check" test schedule while adding new two type of test. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Attachment
> Yes. I added two-phase commit test to "make check" test schedule > while > adding new two type of test. Thank you, committed. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL