ECPG: proposal for new DECLARE STATEMENT

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

ECPG: proposal for new DECLARE STATEMENT

kuroda.hayato@fujitsu.com
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@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.

Best regards

Hayato Kuroda
FUJITSU LIMITED
E-Mail:[hidden email]



DeclareStmt01.patch (70K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ECPG: proposal for new DECLARE STATEMENT

Tomas Vondra-4
Hi,

On Thu, Oct 31, 2019 at 12:29:30PM +0000, [hidden email] 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@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


Reply | Threaded
Open this post in threaded view
|

Re: ECPG: proposal for new DECLARE STATEMENT

Tomas Vondra-4
On Sun, Jan 12, 2020 at 03:52:48AM +0100, Tomas Vondra wrote:
> ...
>
>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.
>

Meh, forgot to attach the file ...


regards

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

minor-fixes.txt (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ECPG: proposal for new DECLARE STATEMENT

David Steele
On 1/11/20 10:41 PM, Tomas Vondra wrote:
> On Sun, Jan 12, 2020 at 03:52:48AM +0100, Tomas Vondra wrote:
>> ...
>>
>> 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.
>>
>
> Meh, forgot to attach the file ...

Any thoughts on Tomas' comments?

A big part of moving a patch forward is keeping the thread active and
answering comments/review.

Regards,
--
-David
[hidden email]


Reply | Threaded
Open this post in threaded view
|

Re: ECPG: proposal for new DECLARE STATEMENT

Daniel Gustafsson
> On 30 Mar 2020, at 18:53, David Steele <[hidden email]> wrote:
>
> On 1/11/20 10:41 PM, Tomas Vondra wrote:
>> On Sun, Jan 12, 2020 at 03:52:48AM +0100, Tomas Vondra wrote:
>>> ...
>>>
>>> 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.
>>>
>> Meh, forgot to attach the file ...
>
> Any thoughts on Tomas' comments?
>
> A big part of moving a patch forward is keeping the thread active and answering comments/review.

This patch has now been silent for quite a while, unless someone is interested
enough to bring it forward it seems about time to close it.

cheers ./daniel

Reply | Threaded
Open this post in threaded view
|

Re: ECPG: proposal for new DECLARE STATEMENT

Michael Meskes-3
> This patch has now been silent for quite a while, unless someone is
> interested
> enough to bring it forward it seems about time to close it.

I am interested but still short on time. I will definitely look into it
as soon as I find some spare minutes.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org