Horton

Thread

Commits

  1. Simplify plpgsql's check for simple expressions.

  2. Redesign the plancache mechanism for more flexibility and efficiency.

  3. Make use of plancache module for SPI plans. In particular, since plpgsql

  1. Is exec_simple_check_node still doing anything?

    Robert Haas <robertmhaas@gmail.com> — 2017-06-20T20:51:36Z

    I'm a little mystified by exec_simple_check_node().  The regression
tests seem not to exercise it.  It can only be reached when
exec_simple_recheck_plan() finds no other reason to reject the plan,
and the only case it seems to reject is the one where there's a
set-returning function buried in there someplace.  But then it seems
like hasTargetSRFs would have been true and we would have given up
before making a plan in the first place.  Of course, that only
protects us when originally forming the plan; they don't account for
later changes -- and the code comments claim that an expression which
was originally simple can become non-simple:

 * It is possible though unlikely for a simple expression to become non-simple
 * (consider for example redefining a trivial view).

But I can't quite figure that one out.  If we're selecting from a
trivial view, then the range table won't be empty and the expression
won't be simple in the first place.  The check for a non-empty range
table didn't exist when this comment was originally added
(95f6d2d20921b7c2dbec29bf2706fd9448208aa6, 2007); it was added in a
subsequent redesign (e6faf910d75027bdce7cd0f2033db4e912592bcc; 2011).
Did that, possibly, remove the last way in which a simple expression
could be could become non-simple?  If so, between that and the new
hasTargetSRFs test, it might now be impossible for
exec_simple_check_node() to fail.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  2. Re: Is exec_simple_check_node still doing anything?

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-06-20T21:44:34Z

    Robert Haas <robertmhaas@gmail.com> writes:
> I'm a little mystified by exec_simple_check_node().
> ...
> Did that, possibly, remove the last way in which a simple expression
> could be could become non-simple?  If so, between that and the new
> hasTargetSRFs test, it might now be impossible for
> exec_simple_check_node() to fail.

I think you might be right.  The other way that I'm aware of that
could cause interesting things to happen is if someone redefines
a SQL function that had been inlined in the originally-compiled
version of the expression.  However, it should be the case that
inline_function() will refuse to inline if the new definition
contains anything "scary", so that the expression as seen by
plpgsql is still simple; any non-simplicity will just be hidden
under a function call.

In fact, I suspect we could get rid of exec_simple_recheck_plan
altogether.  It could use a bit more study, but the empty-rtable
check plus the other checks in exec_simple_check_plan (particularly,
hasAggs, hasWindowFuncs, hasTargetSRFs, hasSubLinks) seem like
they are enough to guarantee that what comes out of the planner
will be "simple".

If I recall things correctly, originally there were only the
post-planning simplicity checks that are now embodied in
exec_simple_recheck_plan/exec_simple_check_node.  I think I added
on the pre-planning checks in exec_simple_check_plan in order to
try to save some planning cycles.  Since the SRF checks were
clearly still necessary at the time, I didn't think hard about
whether any of the other post-planning checks could be got rid of.

			regards, tom lane

  3. Re: Is exec_simple_check_node still doing anything?

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-06-25T17:48:35Z

    I wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> I'm a little mystified by exec_simple_check_node().
>> ...
>> Did that, possibly, remove the last way in which a simple expression
>> could be could become non-simple?  If so, between that and the new
>> hasTargetSRFs test, it might now be impossible for
>> exec_simple_check_node() to fail.

> In fact, I suspect we could get rid of exec_simple_recheck_plan
> altogether.  It could use a bit more study, but the empty-rtable
> check plus the other checks in exec_simple_check_plan (particularly,
> hasAggs, hasWindowFuncs, hasTargetSRFs, hasSubLinks) seem like
> they are enough to guarantee that what comes out of the planner
> will be "simple".

I did some more studying and it definitely seems like
exec_simple_recheck_plan can never fail anymore.  As an experimental
check, converting everything it was testing into Asserts still gets
through check-world.  Accordingly, here's a proposed patch that gets
rid of exec_simple_check_node() and simplifies some of the rest of
the logic.

			regards, tom lane