Обсуждение: Modify pg_stat_get_activity to build a tuplestore

All,

* Robert Haas (robertmhaas@gmail.com) wrote (on the role attributes
* thread):
> I think this patch is too big and does too many things.  It should be
> broken up into small patches which can be discussed and validated
> independently.  The fact that your commit message is incredibly long
> is a sign that there's too much going on here, and that message
> doesn't even cover all of it.

As I mentioned on another thread, you're certainly right about that, and
here's the first broken-out patch, which just changes
pg_stat_get_activity to build and return a tuplestore in a single
function call instead of using the old-style multiple-invokation call
method.  Given the simplicity and the lack of any user-visible change,
and that it's an independently useful change regardless of what happens
with the other changes, I'm planning to move forward with this pretty
quickly, barring concerns, of course.

This is heavily based on the pg_stat_get_wal_senders() implementation,
hat-tip to Itagaki Takahiro for that.

    Thanks!

        Stephen

Stephen Frost wrote:

> As I mentioned on another thread, you're certainly right about that, and
> here's the first broken-out patch, which just changes
> pg_stat_get_activity to build and return a tuplestore in a single
> function call instead of using the old-style multiple-invokation call
> method.  Given the simplicity and the lack of any user-visible change,
> and that it's an independently useful change regardless of what happens
> with the other changes, I'm planning to move forward with this pretty
> quickly, barring concerns, of course.

Um.  Abhijit mentioned to me the idea of implementing function execution
code in fmgr that would allow for calling functions lazily (i.e. stop
calling it once enough rows have been returned).  One of the reasons not
to do that is that most functions use the materialize mode rather than
value-per-call, so the optimization is pointless because the whole
result is computed anyway.  If we change more functions to materialize,
we will make that sort of optimization even more distant.

What is the reason for this change anyway?

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

* Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
> Stephen Frost wrote:
>
> > As I mentioned on another thread, you're certainly right about that, and
> > here's the first broken-out patch, which just changes
> > pg_stat_get_activity to build and return a tuplestore in a single
> > function call instead of using the old-style multiple-invokation call
> > method.  Given the simplicity and the lack of any user-visible change,
> > and that it's an independently useful change regardless of what happens
> > with the other changes, I'm planning to move forward with this pretty
> > quickly, barring concerns, of course.
>
> Um.  Abhijit mentioned to me the idea of implementing function execution
> code in fmgr that would allow for calling functions lazily (i.e. stop
> calling it once enough rows have been returned).  One of the reasons not
> to do that is that most functions use the materialize mode rather than
> value-per-call, so the optimization is pointless because the whole
> result is computed anyway.  If we change more functions to materialize,
> we will make that sort of optimization even more distant.

With pg_stat_get_activity, at least, we already deal with this in a way-
you can pass in the specific pid you want to look at and then we'll just
return the one row that has that pid.

I'm not saying that such an optimization isn't a good idea (I'm all for
it, in fact), but it seems unlikely to help this particular function
much.  Wouldn't we also have to change the API to support that, as there
are functions which expect to be called all the way through and do
something at the end?

I've long wanted to get at information, like if there is a LIMIT or a
WHERE clause or set of conditionals which applies to the results of a
given function and have that exposed to all PLs (pl/pgsql being the big
one there).  That'd be far more valuable than this short-circuiting, not
that I'm against having both, of course.

> What is the reason for this change anyway?

Hrm, guess I wasn't clear enough in the commit message.  The reason is
that the default roles patch turns it into a helper function which takes
arguments and is then called from multiple functions which are exposed
at the SQL level.  That's much simpler when working with a tuplestore
since we can create the tuplestore in the calling function and have the
helper just populate it for us, based on the parameters passed in.
Thanks!
    Stephen