RE: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

From: "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Michael Meskes' <meskes(at)postgresql(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Geoghegan <pg(at)bowt(dot)ie>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: RE: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
Date: 2021-08-10 09:27:46
Message-ID: TYAPR01MB5866200E32E09D1608C21016F5F79@TYAPR01MB5866.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Meskes and any hackers,

> Yes, at least technically. I think DESCRIBE should accept the cached
> connection name, although it won't really matter.

I sought docs too and I found that Pro*C have such a same policy,
so it might be reasonable:

https://docs.oracle.com/en/database/oracle/oracle-database/21/lnpcc/Oracle-dynamic-SQL.html#GUID-0EB50EB7-D4C8-401D-AFCD-340D281711C4

Anyway I revised patches again in the current spec. I separated them into 6 parts:

0001: move "connection = NULL" to top rule. This is per Wang.
0002: adds supporting deallocate statement.
0003: adds supporting describe statement. The above and this are main parts.
0004: adds warning then the connection is overwritten. This is per Horiguchi-san.
0005: adds warning then the connection is overwritten. This is per Horiguchi-san and Paquier.
0006: adds some tests.

0001 is the solution of follows:
https://www.postgresql.org/message-id/OSBPR01MB42141999ED8EFDD4D8FDA42CF2E29%40OSBPR01MB4214.jpnprd01.prod.outlook.com

This bug is caused because "connection = NULL" is missing is missing in some cases, so I force to
substitute NULL in the statement: rule, the top-level in the parse tree.
I also remove the substitution from output.c because such line is overlapped.
If you think this change is too large, I can erase 0001 and add a substitution to the end part of
ECPGCursorStmt rule. That approach is also resolve the bug and impact is very small.

0004 is an optional patch, this is not related with DEALLOCATE and DESCRIBE.
We were discussing about how should work when followings are pre-compiled and executed:

> EXEC SQL AT conn1 DECLARE stmt STATEMENT;
> EXEC SQL AT conn2 EXECUTE stmt INTO ..;

Currently line 2 will execute at conn1 without any warnings (and this is the Oracle's spec) but Horiguchi-san says it is non-obvious.
So I added a precompiler-warning when the above situation.
More discussion might be needed here, but this is not main part.

About 0005, see previous discussion:

> Since we don't allow descriptors with the same name even if they are
> for the different connections, I think we can set the current
> connection if any (which is set either by AT option or statement-bound
> one) to i->connection silently if i->connection is NULL in
> lookup_descriptor(). What do you think about this?

How do you think?

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Attachment Content-Type Size
v04-0001-move-toplevel.patch application/octet-stream 2.4 KB
v04-0002-allow-deallocate.patch application/octet-stream 892 bytes
v04-0003-allow-describe.patch application/octet-stream 4.1 KB
v04-0004-add-warning-in-check_declared_list.patch application/octet-stream 996 bytes
v04-0005-descriptor.patch application/octet-stream 1.5 KB
v04-0006-add-test.patch application/octet-stream 12.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2021-08-10 09:34:28 Re: Bug in huge simplehash
Previous Message Daniel Gustafsson 2021-08-10 09:13:03 Re: add operator ^= to mean not equal (like != and <>)