Re: ECPG: proposal for new DECLARE STATEMENT

Hi,

On Thu, Oct 31, 2019 at 12:29:30PM +0000, kuroda(dot)hayato(at)fujitsu(dot)com wrote:
>Dear hackers,
>
>As declared last month, I propose again the new ECPG grammar, DECLARE STATEMENT.
>This had been committed once, but it removed from PG12 because of
>some problems.
>In this mail, I want to report some problems that previous implementation has,
>produce a new solution, and attach a WIP patch.
>
>[Basic function, Grammar, and Use case]
>This statement will be used for the purpose of designating a connection easily.
>Please see below:
>https://www.postgresql.org/message-id/flat/4E72940DA2BF16479384A86D54D0988A4D80D3C9(at)G01JPEXMBKW04
>The Oracle's manual will also help your understanding:
>https://docs.oracle.com/en/database/oracle/oracle-database/19/lnpcc/embedded-SQL-statements-and-directives.html#GUID-0A30B7B4-BD91-42EA-AACE-2E9CBF7E9C1A
>
>[Issues]
>That's why this feature has been reverted.
>1. The namespace of the identifier was not clear. If you use a same identifier for other SQL statements,
> these interfered each other and statements might be executed at the unexpected connection.
>2. Declaring at the outside of functions was not allowed. This specification is quite different from the other
> declarative statements, so some users might be confused.
> For instance, the following example was rejected.
>```
>EXEC SQL DECLARE stmt STATEMENT;
>
>int
>main()
>{
>...
> EXEC SQL DECLARE cur CURSOR FOR stmt;
>...
>}
>```
>3. These specifications were not compatible with other DBMSs.
>
>[Solutions]
>The namespace is set to be a file unit. This follows other DBMSs.
>When the DECLARE SATATEMENT statement is read, the name, identifier
>and the related connection are recorded.
>And if you use the declared identifier in order to prepare or declare cursor,
>the fourth argument for ECPGdo(it represents the connection) will be overwritten.
>This declaration is enabled only the precompile phase.
>
> [Limitations]
>The declaration must be appeared before using it.
>This also follows Pro*C precompiler.
>
>A WIP patch is attached. Confirm that all ECPG tests have passed,
>however, some documents are not included.
>They will be added later.
>I applied the pgindent as a test, but it might be failed because this is the
>first time for me.
>

I see there were no reviews of this new patch, with the feature
reimplemented after it was reverted from PG12 in September :-(

I'm not an ecpg expert (in fact I've never even used it), so my review
is pretty superficial, but I only found a couple of minor whitespace
issues (adding/removing a line/tab) - see the attached file.

Kuroda-san, you mentioned the patch is WIP. What other bits you think
are missing / need improvement? I see you mentioned some documentation
is missing - I suppose that's one of the missing pieces?

For the record, there were two threads discussing the implementation [1]
and then the revert [2].

[1] https://www.postgresql.org/message-id/flat/1F66B161998C704BABF8989B8A2AC0A313AA41%40G01JPEXMBYT05
[2] https://www.postgresql.org/message-id/flat/TY2PR01MB2443EC8286995378AEB7D9F8F5B10%40TY2PR01MB2443.jpnprd01.prod.outlook.com

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services