Re: EXPLAIN, utility statement parameters, and recent plpgsql changes

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: EXPLAIN, utility statement parameters, and recent plpgsql changes
Date: 2010-01-14 18:40:32
Message-ID: 162867791001141040h4b730efak33358098d5ea6e33@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2010/1/14 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Pavel pointed out here
> http://archives.postgresql.org/pgsql-hackers/2010-01/msg01233.php
> that it no longer works to reference plpgsql variables in EXPLAIN
> statements in plpgsql.  I dug into this a bit, and the code is trying
> to do it but it doesn't quite work.
>
> The issue centers around the behavior of the ParamListInfo data
> structure, which was originally intended to carry only values for a
> fixed set of $1 .. $n parameters (as in PREPARE/EXECUTE for instance).
> This is the structure that carries plpgsql values into a command that's
> executed as a cursor.  To support the recent changes in plgsql parsing,
> I extended that struct to also carry parser hook functions.  The idea is
> that while doing parse analysis of a statement, the parser hook
> functions could capture references to plpgsql variables and turn them
> into Params, which would then reference the data area of the
> ParamListInfo struct at runtime.
>
> This works well enough for regular DML statements, but it falls down for
> EXPLAIN which is a utility statement, because *parse analysis of utility
> statements doesn't do anything*.  EXPLAIN actually does the parse
> analysis of its contained statement at the beginning of execution.
> And that is too late, in the scenario Pavel exhibited.  Why is it too
> late?  Because SPI_cursor_open_internal() intentionally "freezes" the
> ParamListInfo struct after doing initial parsing: what it copies into
> the cursor portal is just a static list of data values without the
> parser hooks (see copyParamList).  This is really necessary because the
> execution of the portal could outlive the function that created the
> cursor, so we can't safely execute its parsing hooks anymore.
>
> So what to do about it?  I can see two basic avenues towards a solution:
>
> 1. Change things so that copyParamList copies enough state into the
> cursor portal so that we can still run the plpgsql parsing hooks during
> cursor execution.  In the worst case this would imply copying *all*
> local variables and parameters of the plpgsql function into the cursor
> portal, plus a lot of names, types, etc.  We could perhaps optimize
> things enough to only copy the values actually referenced, but it still
> seems like possibly a rather nasty performance hit.  And it'd affect not
> only explicit cursors, but every plpgsql for-over-rows construct,
> because those are cursors internally.
>
> 2. Redesign EXPLAIN so that it parses the contained query in the initial
> parsing step; it wouldn't be a simple utility command anymore but a
> hybrid much like DECLARE CURSOR.  I think this would not be very messy.
> The main objection to it is that it doesn't scale to solve the problem
> for other types of utility statements.  Now we don't support parameters
> in other types of utility statements anyway, but it's something we'd
> like to do someday probably.

+1

Pavel
>
> (Of course there are also 3. "Sorry, we're not going to support
> variables in EXPLAIN anymore" and 4. Revert all those parsing fixes
> in plpgsql, but I rejected these solutions out of hand.)
>
> I'm kind of leaning to #2, particularly given that we don't have time
> to expend a great deal of work on this for 8.5.  But I wonder if anyone
> has any comments or alternative ideas.
>
>                        regards, tom lane
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Smith 2010-01-14 18:46:47 Re: Clearing global statistics
Previous Message David Fetter 2010-01-14 18:36:09 Re: mailing list archiver chewing patches