Thread: patch to create system view that lists cursors
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
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
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
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
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
> 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
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
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
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
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