cursors with prepared statements

I have developed a patch that allows declaring cursors over prepared
statements:

DECLARE cursor_name CURSOR FOR prepared_statement_name
[ USING param, param, ... ]

This is an SQL standard feature. ECPG already supports it (with
different internals).

Internally, this just connects existing functionality in different ways,
so it doesn't really introduce anything new.

One point worth pondering is how to pass the parameters of the prepared
statements. The actual SQL standard syntax would be

DECLARE cursor_name CURSOR FOR prepared_statement_name;
OPEN cursor_name USING param, param;

But since we don't have the OPEN statement in direct SQL, it made sense
to me to attach the USING clause directly to the DECLARE statement.

Curiously, the direct EXECUTE statement uses the non-standard syntax

EXECUTE prep_stmt (param, param);

instead of the standard

EXECUTE prep_stmt USING param, param;

I tried to consolidate this. But using

DECLARE c CURSOR FOR p (foo, bar)

leads to parsing conflicts (and looks confusing?), and instead allowing
EXECUTE + USING leads to a mess in the ECPG parser that exhausted me.
So I'm leaving it as is for now and might give supporting EXECUTE +
USING another try later on.

When looking at the patch, some parts will look easier through git diff -w.

And the changes in the ECPG parser are needed because ECPG already
supported that syntax separately, but now it needs to override the rules
from the main parser instead. That stuff has test coverage, fortunately.

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

On Fri, Jun 8, 2018 at 1:12 AM, Peter Eisentraut
<peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> I have developed a patch that allows declaring cursors over prepared
> statements:
>
> DECLARE cursor_name CURSOR FOR prepared_statement_name
> [ USING param, param, ... ]
>
> This is an SQL standard feature. ECPG already supports it (with
> different internals).
>
> Internally, this just connects existing functionality in different ways,
> so it doesn't really introduce anything new.
>
> One point worth pondering is how to pass the parameters of the prepared
> statements. The actual SQL standard syntax would be
>
> DECLARE cursor_name CURSOR FOR prepared_statement_name;
> OPEN cursor_name USING param, param;
>
> But since we don't have the OPEN statement in direct SQL, it made sense
> to me to attach the USING clause directly to the DECLARE statement.
>
> Curiously, the direct EXECUTE statement uses the non-standard syntax
>
> EXECUTE prep_stmt (param, param);
>
> instead of the standard
>
> EXECUTE prep_stmt USING param, param;
>
> I tried to consolidate this. But using
>
> DECLARE c CURSOR FOR p (foo, bar)
>
> leads to parsing conflicts (and looks confusing?), and instead allowing
> EXECUTE + USING leads to a mess in the ECPG parser that exhausted me.
> So I'm leaving it as is for now and might give supporting EXECUTE +
> USING another try later on.
>

Sounds like a reasonable approach. Have you not considered using a
special OPEN syntax because there are some other forms of problems
with it?

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On 6/11/18 09:57, Amit Kapila wrote:
> Sounds like a reasonable approach. Have you not considered using a
> special OPEN syntax because there are some other forms of problems
> with it?

There is no OPEN command in direct SQL. Do you mean whether I have
considered introducing an OPEN command? Yes, but it seems to me that
that would create weird inconsistencies and doesn't seem very useful in
practice.

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

On Mon, Jun 11, 2018 at 9:56 PM, Peter Eisentraut
<peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> On 6/11/18 09:57, Amit Kapila wrote:
>> Sounds like a reasonable approach. Have you not considered using a
>> special OPEN syntax because there are some other forms of problems
>> with it?
>
> There is no OPEN command in direct SQL. Do you mean whether I have
> considered introducing an OPEN command?
>

Yes.

> Yes, but it seems to me that
> that would create weird inconsistencies and doesn't seem very useful in
> practice.
>

Okay, if that doesn't make the job easy, then there is not much use in
pursuing that direction.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On 07/06/18 22:42, Peter Eisentraut wrote:
> I have developed a patch that allows declaring cursors over prepared
> statements:
>
> DECLARE cursor_name CURSOR FOR prepared_statement_name
> [ USING param, param, ... ]
>
> This is an SQL standard feature. ECPG already supports it (with
> different internals).
>
> Internally, this just connects existing functionality in different ways,
> so it doesn't really introduce anything new.
>
> One point worth pondering is how to pass the parameters of the prepared
> statements. The actual SQL standard syntax would be
>
> DECLARE cursor_name CURSOR FOR prepared_statement_name;
> OPEN cursor_name USING param, param;
>
> But since we don't have the OPEN statement in direct SQL, it made sense
> to me to attach the USING clause directly to the DECLARE statement.

Hmm. I'm not excited about adding PostgreSQL-extensions to the SQL
standard. It's confusing, and risks conflicting with future additions to
the standard. ECPG supports the actual standard syntax, with OPEN,
right? So this wouldn't be consistent with ECPG, either.

> Curiously, the direct EXECUTE statement uses the non-standard syntax
>
> EXECUTE prep_stmt (param, param);
>
> instead of the standard
>
> EXECUTE prep_stmt USING param, param;
>
> I tried to consolidate this. But using
>
> DECLARE c CURSOR FOR p (foo, bar)
>
> leads to parsing conflicts (and looks confusing?),

How about

DECLARE c CURSOR FOR EXECUTE p (foo, bar)

? As a user, I'm already familiar with the "EXECUTE p (foo, bar)"
syntax, so that's what I would intuitively try to use with DECLARE as
well. In fact, I think I tried doing just that once, and was
disappointed that it didn't work.

> and instead allowing
> EXECUTE + USING leads to a mess in the ECPG parser that exhausted me.
> So I'm leaving it as is for now and might give supporting EXECUTE +
> USING another try later on.

The attached patch seems to do the trick, of allowing EXECUTE + USING.
I'm not sure this is worth the trouble, though, since EXECUTE as a plain
SQL command is a PostgreSQL-extension anyway.

This also adds a test case for the existing "EXECUTE <stmt> (<params>)"
syntax in ECPG. The current ECPG parsing of that is actually a bit
weird, it allows "EXECUTE stmt (:param1) USING :param2", which seems
unintentional. This patch rejects that syntax.

- Heikki

On 11.07.18 19:07, Heikki Linnakangas wrote:
>> One point worth pondering is how to pass the parameters of the prepared
>> statements. The actual SQL standard syntax would be
>>
>> DECLARE cursor_name CURSOR FOR prepared_statement_name;
>> OPEN cursor_name USING param, param;
>>
>> But since we don't have the OPEN statement in direct SQL, it made sense
>> to me to attach the USING clause directly to the DECLARE statement.
>
> Hmm. I'm not excited about adding PostgreSQL-extensions to the SQL
> standard.

Isn't that what we do all the time?

> It's confusing, and risks conflicting with future additions to
> the standard. ECPG supports the actual standard syntax, with OPEN,
> right? So this wouldn't be consistent with ECPG, either.

It would be consistent for the case of no parameters.

>> Curiously, the direct EXECUTE statement uses the non-standard syntax
>>
>> EXECUTE prep_stmt (param, param);
>>
>> instead of the standard
>>
>> EXECUTE prep_stmt USING param, param;
>>
>> I tried to consolidate this. But using
>>
>> DECLARE c CURSOR FOR p (foo, bar)
>>
>> leads to parsing conflicts (and looks confusing?),
>
> How about
>
> DECLARE c CURSOR FOR EXECUTE p (foo, bar)

That's not the standard syntax for the case of no parameters.

> The attached patch seems to do the trick, of allowing EXECUTE + USING.
> I'm not sure this is worth the trouble, though, since EXECUTE as a plain
> SQL command is a PostgreSQL-extension anyway.

I think it's a PostgreSQL extension that we allow just about anything to
be executed directly. So we should still use the standard syntax either
way. It would be weird if EXECUTE or any other command had different
syntax in direct SQL, ECPG, PL/pgSQL, etc. We have some differences
already, but we shouldn't create more.

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

On Mon, Jul 16, 2018 at 8:56 AM, Peter Eisentraut
<peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
>> The attached patch seems to do the trick, of allowing EXECUTE + USING.
>> I'm not sure this is worth the trouble, though, since EXECUTE as a plain
>> SQL command is a PostgreSQL-extension anyway.
>
> I think it's a PostgreSQL extension that we allow just about anything to
> be executed directly. So we should still use the standard syntax either
> way. It would be weird if EXECUTE or any other command had different
> syntax in direct SQL, ECPG, PL/pgSQL, etc. We have some differences
> already, but we shouldn't create more.

Hmm. Your proposal to attach a USING clause to DECLARE .. CURSOR FOR
rather than inventing an OPEN command is an argument for a PostgreSQL
syntax extension, but your proposal to write DECLARE .. CURSOR FOR
rather than DECLARE .. CURSOR FOR EXECUTE is an argument for standard
syntax over and against a PostgreSQL extension.

That sounds a little contradictory, but I think I agree with it. If
we allow a USING clause for DECLARE .. CURSOR FOR, that doesn't
prevent somebody from inventing an OPEN command in the future. As
part of introducing such an OPEN command, DECLARE .. CURSOR FOR could
be made not to fail if the prepared statement has parameters but no
USING commands. On the other hand, if we insist on injecting the word
EXECUTE into the syntax as Heikki proposes, that's purely and simply
an incompatibility with the standard's syntax, as well as with what
ECPG already does.

So +1 for your position.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 16/07/18 15:56, Peter Eisentraut wrote:
> On 11.07.18 19:07, Heikki Linnakangas wrote:
>> It's confusing, and risks conflicting with future additions to
>> the standard. ECPG supports the actual standard syntax, with OPEN,
>> right? So this wouldn't be consistent with ECPG, either.
>
> It would be consistent for the case of no parameters.

True. Except that if I understand correctly, in the standard syntax you
still need to use OPEN after the DECLARE CURSOR, even when there are no
parameters.

>>> Curiously, the direct EXECUTE statement uses the non-standard syntax
>>>
>>> EXECUTE prep_stmt (param, param);
>>>
>>> instead of the standard
>>>
>>> EXECUTE prep_stmt USING param, param;
>>>
>>> I tried to consolidate this. But using
>>>
>>> DECLARE c CURSOR FOR p (foo, bar)
>>>
>>> leads to parsing conflicts (and looks confusing?),
>>
>> How about
>>
>> DECLARE c CURSOR FOR EXECUTE p (foo, bar)
>
> That's not the standard syntax for the case of no parameters.

My thinking here is that "DECLARE c CURSOR FOR <statement>" is standard
syntax. And we already have "EXECUTE p (foo, bar)" as a form of
statement, along with "SELECT ...", "EXPLAIN ..." and so forth. Allowing
"DECLARE c CURSOR FOR EXECUTE p (foo, bar)" would not introduce a new
syntax, it would just allow the existing two commands, DECLARE CURSOR,
and EXECUTE, to be used together.

- Heikki

> On Wed, Jul 18, 2018 at 10:27 AM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>
> On 16/07/18 15:56, Peter Eisentraut wrote:
> > On 11.07.18 19:07, Heikki Linnakangas wrote:
> >> It's confusing, and risks conflicting with future additions to
> >> the standard. ECPG supports the actual standard syntax, with OPEN,
> >> right? So this wouldn't be consistent with ECPG, either.
> >
> > It would be consistent for the case of no parameters.
>
> True. Except that if I understand correctly, in the standard syntax you
> still need to use OPEN after the DECLARE CURSOR, even when there are no
> parameters.
>
> >>> Curiously, the direct EXECUTE statement uses the non-standard syntax
> >>>
> >>> EXECUTE prep_stmt (param, param);
> >>>
> >>> instead of the standard
> >>>
> >>> EXECUTE prep_stmt USING param, param;
> >>>
> >>> I tried to consolidate this. But using
> >>>
> >>> DECLARE c CURSOR FOR p (foo, bar)
> >>>
> >>> leads to parsing conflicts (and looks confusing?),
> >>
> >> How about
> >>
> >> DECLARE c CURSOR FOR EXECUTE p (foo, bar)
> >
> > That's not the standard syntax for the case of no parameters.
>
> My thinking here is that "DECLARE c CURSOR FOR <statement>" is standard
> syntax. And we already have "EXECUTE p (foo, bar)" as a form of
> statement, along with "SELECT ...", "EXPLAIN ..." and so forth. Allowing
> "DECLARE c CURSOR FOR EXECUTE p (foo, bar)" would not introduce a new
> syntax, it would just allow the existing two commands, DECLARE CURSOR,
> and EXECUTE, to be used together.

This patch went through the last few commitfests without any noticeable
activity. Both suggested patches are still good (can be applied and passed all
the tests, except the minor text mismatch in the original one), but looks like
the discussion stopped right in the middle. Are there any more opinions about
OPEN vs DECLARE .. CURSOR FOR here or any other plans about the patch?

> On Thu, Nov 22, 2018 at 11:11 AM Dmitry Dolgov <9erthalion6(at)gmail(dot)com> wrote:
>
> > On Wed, Jul 18, 2018 at 10:27 AM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> >
> > On 16/07/18 15:56, Peter Eisentraut wrote:
> > > On 11.07.18 19:07, Heikki Linnakangas wrote:
> > >> It's confusing, and risks conflicting with future additions to
> > >> the standard. ECPG supports the actual standard syntax, with OPEN,
> > >> right? So this wouldn't be consistent with ECPG, either.
> > >
> > > It would be consistent for the case of no parameters.
> >
> > True. Except that if I understand correctly, in the standard syntax you
> > still need to use OPEN after the DECLARE CURSOR, even when there are no
> > parameters.
> >
> > >>> Curiously, the direct EXECUTE statement uses the non-standard syntax
> > >>>
> > >>> EXECUTE prep_stmt (param, param);
> > >>>
> > >>> instead of the standard
> > >>>
> > >>> EXECUTE prep_stmt USING param, param;
> > >>>
> > >>> I tried to consolidate this. But using
> > >>>
> > >>> DECLARE c CURSOR FOR p (foo, bar)
> > >>>
> > >>> leads to parsing conflicts (and looks confusing?),
> > >>
> > >> How about
> > >>
> > >> DECLARE c CURSOR FOR EXECUTE p (foo, bar)
> > >
> > > That's not the standard syntax for the case of no parameters.
> >
> > My thinking here is that "DECLARE c CURSOR FOR <statement>" is standard
> > syntax. And we already have "EXECUTE p (foo, bar)" as a form of
> > statement, along with "SELECT ...", "EXPLAIN ..." and so forth. Allowing
> > "DECLARE c CURSOR FOR EXECUTE p (foo, bar)" would not introduce a new
> > syntax, it would just allow the existing two commands, DECLARE CURSOR,
> > and EXECUTE, to be used together.
>
> This patch went through the last few commitfests without any noticeable
> activity. Both suggested patches are still good (can be applied and passed all
> the tests, except the minor text mismatch in the original one), but looks like
> the discussion stopped right in the middle. Are there any more opinions about
> OPEN vs DECLARE .. CURSOR FOR here or any other plans about the patch?

I hope it's not another abandoned patch, but due to lack of response I'm
marking it as returned with feedback. If someone didn't see my previous
inquiry and ready to continue working on this patch - feel free to change it
back.