Thread: 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.
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().
Attaching the ECPG program for reference.
Thanks,
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.
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().
Attaching the ECPG program for reference.
Thanks,
Attachment
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
On Thu, Mar 7, 2019 at 9:56 AM Michael Meskes <meskes@postgresql.org> 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
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
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
Attachment
> 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
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
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
Attachment
Hi Kurokawa-san I reviewd it. It's ok. I also confirm there is no same bug. Regards Ryo Matsumura
On Wed, Mar 13, 2019 at 04:35:48AM +0000, 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 <meskes@postgresql.org>
Date: Fri Mar 15 22:35:24 2019 +0100
Use correct connection name variable in ecpglib.
Fixed-by: Kuroda-san <kuroda.hayato@jp.fujitsu.com>
--
Bruce Momjian <bruce@momjian.us> 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 +