Thread: patch to create system view that lists cursors

patch to create system view that lists cursors

From
Joachim Wieland
Date:
Attached is a patch to create a new system view pg_cursors to list all
available cursors to the current session and transaction.

The patch itself is quite analogous to the patch for pg_prepared_statements
I submitted in December.

The attributes is_holdable, is_binary and is_scrollable are exposed in the
view.

There's a TODO item that only talks about WITH HOLD cursors, however the
proposed system view lists WITHOUT HOLD cursors as well.

   o %Allow pooled connections to list all open WITH HOLD cursors

     Because WITH HOLD cursors exist outside transactions, this allows
     them to be listed so they can be closed.


I noticed that there is no regression test for binary cursors. Should there
be one? I now create one to check the view but don't actually query it.


Joachim

Attachment

Re: patch to create system view that lists cursors

From
Neil Conway
Date:
On Thu, 2006-01-12 at 10:51 +0100, Joachim Wieland wrote:
> Attached is a patch to create a new system view pg_cursors to list all
> available cursors to the current session and transaction.

+ /* loop until we find a named portal or hit the end of the list */
+ while ((hentry = hash_seq_search(hash_seq)) != NULL)
+ {
+     portal = hentry->portal;
+     /* there can be a named portal created by CreateNewPortal, its
name
+      * will be "<unnamed portal n>" (see CreateNewPortal function in
this
+      * file). Those have a status of PORTAL_NEW. The status of cursors
is
+      * PORTAL_READY however. */
+     if (portal->status != PORTAL_READY)
+         continue;
+     if (portal->name[0] != '\0')
+         break;
+ }

I think it is worth distinguishing more clearly between portals that
should be displayed to the user and those that should not (which might
be labelled "internal cursors", perhaps). The tests above seem fairly
ad-hoc.

Barring any objections, I'll implement the above and apply the revised
patch tomorrow.

-Neil



Re: patch to create system view that lists cursors

From
Tom Lane
Date:
Neil Conway <neilc@samurai.com> writes:
> The tests above seem fairly ad-hoc.

No kidding.  But what do you think the correct test is?  The comment's
claim that stuff named "<unnamed cursor>" should be suppressed seems
wrong to begin with, as those are perfectly good cursors.  I'm also
unconvinced that a test on portal->status is a good idea, as I doubt
that ACTIVE should be excluded, and I'm not sure that DONE or FAILED
should be either, and NEW is probably a non-issue because you'll never
see it from within a running command.

What is the point of having this code discriminate against any portals
whatever?

            regards, tom lane

Re: patch to create system view that lists cursors

From
Neil Conway
Date:
On Thu, 2006-01-12 at 19:27 -0500, Tom Lane wrote:
> No kidding.  But what do you think the correct test is?  The comment's
> claim that stuff named "<unnamed cursor>" should be suppressed seems
> wrong to begin with, as those are perfectly good cursors.

Well, I suggested to Joachim offlist that these shouldn't be listed. I
think the intent of the feature is to list the available *cursors*, not
all the available Portals. Why list Portals that cannot be directly
manipulated by the user? It would also mean that this would produce
unexpected results: "PREPARE foo AS SELECT * FROM pg_cursors; EXECUTE
foo".

-Neil



Re: patch to create system view that lists cursors

From
Tom Lane
Date:
Neil Conway <neilc@samurai.com> writes:
> Why list Portals that cannot be directly manipulated by the user?

Define "directly manipulated".  AFAIR there isn't any particular
distinction between portals that got made by DECLARE CURSOR and those
that got made by Bind; you can use either EXECUTE or Fetch for either.
Also, a portal that has gone into ERROR state should still be listed,
IMHO, because the behavior the user will expect is that it's there until
he CLOSEs it.

> It would also mean that this would produce unexpected results:
> "PREPARE foo AS SELECT * FROM pg_cursors; EXECUTE foo".

Unexpected in what sense?

            regards, tom lane

Re: patch to create system view that lists cursors

From
Christopher Kings-Lynne
Date:
> I think it is worth distinguishing more clearly between portals that
> should be displayed to the user and those that should not (which might
> be labelled "internal cursors", perhaps). The tests above seem fairly
> ad-hoc.

With all this system view love going on, is there any point having a
'pg_savepoints' view to see what savepoints you've made?

Chris


Re: patch to create system view that lists cursors

From
Tom Lane
Date:
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
> With all this system view love going on, is there any point having a
> 'pg_savepoints' view to see what savepoints you've made?

Probably not, seeing that one of the main situations where you'd want to
know that would be after an error, and SELECT isn't going to work in
that context.

            regards, tom lane

Re: patch to create system view that lists cursors

From
Neil Conway
Date:
On Thu, 2006-01-12 at 19:51 -0500, Tom Lane wrote:
> Neil Conway <neilc@samurai.com> writes:
> > It would also mean that this would produce unexpected results:
> > "PREPARE foo AS SELECT * FROM pg_cursors; EXECUTE foo".
>
> Unexpected in what sense?

"Unexpected" in the sense that the user would have no reason to expect
an "<unnamed portal n>" row in the pg_cursors view, merely because we
happen to create a portal internally to implement the EXECUTE command.

I think the view should include the portals created by DECLARE CURSOR
and "Bind" protocol messages, but should not include the unnamed portal
or any other portals that are created internally as part of the
implementation of other commands (e.g. EXECUTE). I'm not sure how to
handle SPI: developers using SPI would expect to find their portals in
the view, but those using SPI indirectly (e.g. via PL/foo) would
probably find the clutter surprising. I'd say we need to include SPI
portals in the view as well.

-Neil



Re: patch to create system view that lists cursors

From
Neil Conway
Date:
On Sun, 2006-01-15 at 17:57 -0500, Neil Conway wrote:
> I think the view should include the portals created by DECLARE CURSOR
> and "Bind" protocol messages, but should not include the unnamed portal
> or any other portals that are created internally as part of the
> implementation of other commands (e.g. EXECUTE). I'm not sure how to
> handle SPI: developers using SPI would expect to find their portals in
> the view, but those using SPI indirectly (e.g. via PL/foo) would
> probably find the clutter surprising. I'd say we need to include SPI
> portals in the view as well.

Attached is a revised version of Joachim's patch that implements this.
Cursors created via SPI are part of the view, which produces somewhat
unexpected results when querying the view from a procedural language as
noted above, but I suppose it's the best compromise.

The documentation still needs some work. Barring any objections, I'll
fix that and a few other minor issues and then apply the patch tomorrow.

-Neil


Attachment

Re: patch to create system view that lists cursors

From
Neil Conway
Date:
On Tue, 2006-01-17 at 01:28 -0500, Neil Conway wrote:
> Attached is a revised version of Joachim's patch that implements this.
> Cursors created via SPI are part of the view, which produces somewhat
> unexpected results when querying the view from a procedural language as
> noted above, but I suppose it's the best compromise.

Applied to HEAD.

-Neil