committing inside cursor loop

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

committing inside cursor loop

Peter Eisentraut-6
Here is a patch that allows COMMIT inside cursor loops in PL/pgSQL.  As
alluded to in earlier threads, this is done by converting such cursors
to holdable automatically.  A special flag "auto-held" marks such
cursors, so we know to clean them up on exceptions.

This is currently only for PL/pgSQL, but the same technique could be
applied easily to other PLs.

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

v1-0001-PL-pgSQL-Allow-committing-inside-cursor-loop.patch (14K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: committing inside cursor loop

Tom Lane-2
Peter Eisentraut <[hidden email]> writes:
> Here is a patch that allows COMMIT inside cursor loops in PL/pgSQL.  As
> alluded to in earlier threads, this is done by converting such cursors
> to holdable automatically.  A special flag "auto-held" marks such
> cursors, so we know to clean them up on exceptions.

I haven't really read this patch, but this bit jumped out at me:

+    Inside a cursor loop, <command>ROLLBACK</command> is not allowed.  (That
+    would have to roll back the cursor query, thus invalidating the loop.)

Say what?  That seems to translate into "we have lost the ability to
deal with errors".  I don't think this is really what people are hoping
to get out of the transactional-procedure facility.

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: committing inside cursor loop

Simon Riggs
In reply to this post by Peter Eisentraut-6
On 20 February 2018 at 14:11, Peter Eisentraut
<[hidden email]> wrote:
> Here is a patch that allows COMMIT inside cursor loops in PL/pgSQL.  As
> alluded to in earlier threads, this is done by converting such cursors
> to holdable automatically.  A special flag "auto-held" marks such
> cursors, so we know to clean them up on exceptions.
>
> This is currently only for PL/pgSQL, but the same technique could be
> applied easily to other PLs.

Amazingly clean, looks great.

I notice that PersistHoldablePortal() does ExecutorRewind().

In most cases, the cursor loop doesn't ever rewind. So it would be
good if we could pass a parameter that skips the rewind since it will
never be needed and causes a performance hit. What I imagine is we can
just persist the as-yet unfetched portion of the cursor from the
current row onwards, rather than rewind and store the whole cursor
result.

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

Reply | Threaded
Open this post in threaded view
|

Re: committing inside cursor loop

Simon Riggs
In reply to this post by Tom Lane-2
On 20 February 2018 at 14:45, Tom Lane <[hidden email]> wrote:

> Peter Eisentraut <[hidden email]> writes:
>> Here is a patch that allows COMMIT inside cursor loops in PL/pgSQL.  As
>> alluded to in earlier threads, this is done by converting such cursors
>> to holdable automatically.  A special flag "auto-held" marks such
>> cursors, so we know to clean them up on exceptions.
>
> I haven't really read this patch, but this bit jumped out at me:
>
> +    Inside a cursor loop, <command>ROLLBACK</command> is not allowed.  (That
> +    would have to roll back the cursor query, thus invalidating the loop.)

Hmm, why?

Rollback would only invalidate the cursor if it hadn't yet hit a
Commit. But if you did a commit, then the cursor would become holdable
and you could happily continue reading through the loop even after the
rollback.

So if Commit changes a pinned portal into a holdable cursor, we just
make Rollback do that also. Obviously only for pinned portals, i.e.
the query/ies whose results we are currently looping through.

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

Reply | Threaded
Open this post in threaded view
|

Re: committing inside cursor loop

Ildus Kurbangaliev
In reply to this post by Peter Eisentraut-6
On Tue, 20 Feb 2018 09:11:50 -0500
Peter Eisentraut <[hidden email]> wrote:

> Here is a patch that allows COMMIT inside cursor loops in PL/pgSQL.
> As alluded to in earlier threads, this is done by converting such
> cursors to holdable automatically.  A special flag "auto-held" marks
> such cursors, so we know to clean them up on exceptions.
>
> This is currently only for PL/pgSQL, but the same technique could be
> applied easily to other PLs.
>

Hi,
The patch still applies, tests passed. The code looks nice,
documentation and tests are there.

Although as was discussed before it seems inconsistent without ROLLBACK
support. There was a little discussion about it, but no replies. Maybe
the status of the patch should be changed to 'Waiting on author' until
the end of discussion.

--
---
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

Reply | Threaded
Open this post in threaded view
|

Re: committing inside cursor loop

Peter Eisentraut-6
On 3/6/18 07:48, Ildus Kurbangaliev wrote:
> Although as was discussed before it seems inconsistent without ROLLBACK
> support. There was a little discussion about it, but no replies. Maybe
> the status of the patch should be changed to 'Waiting on author' until
> the end of discussion.

I'm wondering what the semantics of it should be.

For example, consider this:

drop table test1;
create table test1 (a int, b int);
insert into test1 values (1, 11), (2, 22), (3, 33);

do
language plpgsql
$$
declare
  x int;
begin
  for x in update test1 set b = b + 1 where b > 20 returning a loop
    raise info 'x = %', x;
    if x = 2 then
       rollback;
    end if;
  end loop;
end;
$$;

The ROLLBACK call in the first loop iteration undoes the UPDATE command
that drives the loop.  Is it then sensible to continue the loop?

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

Reply | Threaded
Open this post in threaded view
|

Re: committing inside cursor loop

Ildus Kurbangaliev
On Tue, 13 Mar 2018 11:08:36 -0400
Peter Eisentraut <[hidden email]> wrote:

> On 3/6/18 07:48, Ildus Kurbangaliev wrote:
> > Although as was discussed before it seems inconsistent without
> > ROLLBACK support. There was a little discussion about it, but no
> > replies. Maybe the status of the patch should be changed to
> > 'Waiting on author' until the end of discussion.  
>
> I'm wondering what the semantics of it should be.
>
> For example, consider this:
>
> drop table test1;
> create table test1 (a int, b int);
> insert into test1 values (1, 11), (2, 22), (3, 33);
>
> do
> language plpgsql
> $$
> declare
>   x int;
> begin
>   for x in update test1 set b = b + 1 where b > 20 returning a loop
>     raise info 'x = %', x;
>     if x = 2 then
>        rollback;
>     end if;
>   end loop;
> end;
> $$;
>
> The ROLLBACK call in the first loop iteration undoes the UPDATE
> command that drives the loop.  Is it then sensible to continue the
> loop?
>

I think that in the first place ROLLBACK was prohibited because of cases
like this, but it seems to safe to continue the loop when portal
strategy is PORTAL_ONE_SELECT.

--
---
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

Reply | Threaded
Open this post in threaded view
|

Re: committing inside cursor loop

Peter Eisentraut-6
On 3/14/18 08:05, Ildus Kurbangaliev wrote:
>> The ROLLBACK call in the first loop iteration undoes the UPDATE
>> command that drives the loop.  Is it then sensible to continue the
>> loop?
>>
> I think that in the first place ROLLBACK was prohibited because of cases
> like this, but it seems to safe to continue the loop when portal
> strategy is PORTAL_ONE_SELECT.

Maybe, but even plain SELECT commands can have side effects.

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

Reply | Threaded
Open this post in threaded view
|

Re: committing inside cursor loop

Peter Eisentraut-6
On 3/19/18 20:40, Peter Eisentraut wrote:

> On 3/14/18 08:05, Ildus Kurbangaliev wrote:
>>> The ROLLBACK call in the first loop iteration undoes the UPDATE
>>> command that drives the loop.  Is it then sensible to continue the
>>> loop?
>>>
>> I think that in the first place ROLLBACK was prohibited because of cases
>> like this, but it seems to safe to continue the loop when portal
>> strategy is PORTAL_ONE_SELECT.
>
> Maybe, but even plain SELECT commands can have side effects.
Here is an updated patch that supports the ROLLBACK case as well, and
prevents holding portals with a strategy other than PORTAL_ONE_SELECT.

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

v2-0001-PL-pgSQL-Allow-committing-inside-cursor-loop.patch (22K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: committing inside cursor loop

Ildus Kurbangaliev-2
The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            tested, passed

I have checked new version. Although I can miss something,  the patch looks good to me.

The new status of this patch is: Ready for Committer
Reply | Threaded
Open this post in threaded view
|

Re: committing inside cursor loop

Peter Eisentraut-6
On 3/28/18 11:34, Ildus Kurbangaliev wrote:
> The following review has been posted through the commitfest application:
> make installcheck-world:  tested, passed
> Implements feature:       tested, passed
> Spec compliant:           tested, passed
> Documentation:            tested, passed
>
> I have checked new version. Although I can miss something,  the patch looks good to me.
>
> The new status of this patch is: Ready for Committer

Committed, thanks!

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

Reply | Threaded
Open this post in threaded view
|

Re: committing inside cursor loop

Andrew Gierth
>>>>> "Peter" == Peter Eisentraut <[hidden email]> writes:

 Peter> Committed, thanks!

So... what is a pl/* that does _not_ use pinned cursors for cursor loops
supposed to do in this case?

--
Andrew (irc:RhodiumToad)

Reply | Threaded
Open this post in threaded view
|

Re: committing inside cursor loop

Peter Eisentraut-6
On 3/29/18 07:37, Andrew Gierth wrote:
>>>>>> "Peter" == Peter Eisentraut <[hidden email]> writes:
>
>  Peter> Committed, thanks!
>
> So... what is a pl/* that does _not_ use pinned cursors for cursor loops
> supposed to do in this case?

Other than maybe using pinned cursors, one would have to look at the
whole picture and see what makes sense.

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

Reply | Threaded
Open this post in threaded view
|

Re: committing inside cursor loop

Andrew Gierth
>>>>> "Peter" == Peter Eisentraut <[hidden email]> writes:

 >> So... what is a pl/* that does _not_ use pinned cursors for cursor
 >> loops supposed to do in this case?

 Peter> Other than maybe using pinned cursors, one would have to look at
 Peter> the whole picture and see what makes sense.

Never mind, I was overlooking the obvious: explicitly specifying
CURSOR_OPT_HOLD is sufficient for me. (Can't really use pinned cursors
because they might not be unpinned fast enough due to timing of garbage
collection, which would lead to spurious errors.)

--
Andrew (irc:RhodiumToad)

Previous Thread Next Thread