Re: ECPG regression with DECLARE STATEMENT support
On Wed, Mar 13, 2019 at 04:35:48AM +, Matsumura, Ryo wrote: > Hi Kurokawa-san > > I reviewd it. It's ok. > I also confirm there is no same bug. FYI, this was applied a few weeks ago: Author: Michael Meskes Date: Fri Mar 15 22:35:24 2019 +0100 Use correct connection name variable in ecpglib. Fixed-by: Kuroda-san -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
RE: ECPG regression with DECLARE STATEMENT support
Hi Kurokawa-san I reviewd it. It's ok. I also confirm there is no same bug. Regards Ryo Matsumura
RE: ECPG regression with DECLARE STATEMENT support
Dear Matsumura-san, > I think that the 2nd argument of following ecpg_init() must be > real_connection_name. > Is it right? Yes, I think it should be real_connection_name for raising correct error message. This is also an leak of my code and I attached a patch. Best Regards, Hayato Kuroda Fujitsu LIMITED fix_argument.patch Description: fix_argument.patch
RE: ECPG regression with DECLARE STATEMENT support
Hi Kuroda-san I think that the 2nd argument of following ecpg_init() must be real_connection_name. Is it right? ECPGdeallocate(int lineno, int c, const char *connection_name, const char *name) : con = ecpg_get_connection(real_connection_name); if (!ecpg_init(con, connection_name, lineno)) ^^^ Regards Ryo Matsumura
Re: ECPG regression with DECLARE STATEMENT support
> I attached a simple bug-fixing patch. I'm not happy with the situation, but don't see a better solution either. Therefore I committed the change to get rid of the regression. Thanks. 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
RE: ECPG regression with DECLARE STATEMENT support
Dear Rushabh, Michael, I attached a simple bug-fixing patch. Could you review it? An added logic is: 1. Send a close statement to a backend process regardless of the existence of a cursor. 2. If ecpg_do function returns false, raise “cursor is invalid” error. 3. Remove cursor from application. I already checked this patch passes regression tests and Rushabh’s test code. Best Regards, Hayato Kuroda Fujitsu LIMITED DeclareStmt_fix_close_bug.patch Description: DeclareStmt_fix_close_bug.patch
RE: ECPG regression with DECLARE STATEMENT support
Dear Rushabh, Michael, Thank you for reporting. I understood bugs had be embed in ecpg by me. I start checking codes and investigating solutions and other errors. Best Regards, Hayato Kuroda Fujitsu LIMITED
Re: ECPG regression with DECLARE STATEMENT support
On Thu, Mar 7, 2019 at 9:56 AM Michael Meskes wrote: > Hi, > > > Commit bd7c95f0c1a38becffceb3ea7234d57167f6d4bf add DECLARE > > STATEMENT support to ECPG. This introduced the new rule > > for EXEC SQL CLOSE cur and with that it gets transformed into > > ECPGclose(). > > > > Now prior to the above commit, someone can declare the > > cursor in the SQL statement and "CLOSE cur_name" can be > > also, execute as a normal statement. > > That still works, the difference in your test case is that the DECLARE > statement is prepared. > > > Example: > > > > EXEC SQL PREPARE cur_query FROM "DECLARE cur1 CURSOR WITH HOLD FOR > > SELECT count(*) FROM pg_class"; > > EXEC SQL PREPARE fetch_stmt FROM "FETCH next FROM cur1"; > > EXEC SQL EXECUTE cur_query; > > EXEC SQL EXECUTE fetch_stmt INTO :c; > > EXEC SQL CLOSE cur1; > > > > With commit bd7c95f0c1, "EXEC SQL CLOSE cur1" will fail > > and throw an error "sqlcode -245 The cursor is invalid". > > > > I think the problem here is ECPGclose(), tries to find the > > cursor into "connection->cursor_stmts" and if it doesn't > > find it there, just throws an error. Maybe require fix > > into ECPGclose() - rather than throwing an error continue > > executing statement "CLOSE cur_name" with ecpg_do(). > > The problem as I see it is that the cursor is known to the backend but > not the library. Exactly. So maybe we should add logic into ECPGclose() if it doesn't find a cursor in the library - just send a query to the backend rather than throwing an error. > Takaheshi-san, Hayato-san, any idea how to improve the > situation to not error out on statements that used to work? > > 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 > > > -- Rushabh Lathia
Re: ECPG regression with DECLARE STATEMENT support
Hi, > Commit bd7c95f0c1a38becffceb3ea7234d57167f6d4bf add DECLARE > STATEMENT support to ECPG. This introduced the new rule > for EXEC SQL CLOSE cur and with that it gets transformed into > ECPGclose(). > > Now prior to the above commit, someone can declare the > cursor in the SQL statement and "CLOSE cur_name" can be > also, execute as a normal statement. That still works, the difference in your test case is that the DECLARE statement is prepared. > Example: > > EXEC SQL PREPARE cur_query FROM "DECLARE cur1 CURSOR WITH HOLD FOR > SELECT count(*) FROM pg_class"; > EXEC SQL PREPARE fetch_stmt FROM "FETCH next FROM cur1"; > EXEC SQL EXECUTE cur_query; > EXEC SQL EXECUTE fetch_stmt INTO :c; > EXEC SQL CLOSE cur1; > > With commit bd7c95f0c1, "EXEC SQL CLOSE cur1" will fail > and throw an error "sqlcode -245 The cursor is invalid". > > I think the problem here is ECPGclose(), tries to find the > cursor into "connection->cursor_stmts" and if it doesn't > find it there, just throws an error. Maybe require fix > into ECPGclose() - rather than throwing an error continue > executing statement "CLOSE cur_name" with ecpg_do(). The problem as I see it is that the cursor is known to the backend but not the library. Takaheshi-san, Hayato-san, any idea how to improve the situation to not error out on statements that used to work? 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
