pgsql: Rearrange execution of PARAM_EXTERN Params for plpgsql's benefit

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Rearrange execution of PARAM_EXTERN Params for plpgsql's benefit
Date: 2017-12-21 17:57:59
Message-ID: E1eS56V-0006U0-VP@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Rearrange execution of PARAM_EXTERN Params for plpgsql's benefit.

This patch does three interrelated things:

* Create a new expression execution step type EEOP_PARAM_CALLBACK
and add the infrastructure needed for add-on modules to generate that.
As discussed, the best control mechanism for that seems to be to add
another hook function to ParamListInfo, which will be called by
ExecInitExpr if it's supplied and a PARAM_EXTERN Param is found.
For stand-alone expressions, we add a new entry point to allow the
ParamListInfo to be specified directly, since it can't be retrieved
from the parent plan node's EState.

* Redesign the API for the ParamListInfo paramFetch hook so that the
ParamExternData array can be entirely virtual. This also lets us get rid
of ParamListInfo.paramMask, instead leaving it to the paramFetch hook to
decide which param IDs should be accessible or not. plpgsql_param_fetch
was already doing the identical masking check, so having callers do it too
seemed redundant. While I was at it, I added a "speculative" flag to
paramFetch that the planner can specify as TRUE to avoid unwanted failures.
This solves an ancient problem for plpgsql that it couldn't provide values
of non-DTYPE_VAR variables to the planner for fear of triggering premature
"record not assigned yet" or "field not found" errors during planning.

* Rework plpgsql to get rid of the need for "unshared" parameter lists,
by dint of turning the single ParamListInfo per estate into a nearly
read-only data structure that doesn't instantiate any per-variable data.
Instead, the paramFetch hook controls access to per-variable data and can
make the right decisions on the fly, replacing the cases that we used to
need multiple ParamListInfos for. This might perhaps have been a
performance loss on its own, but by using a paramCompile hook we can
bypass plpgsql_param_fetch entirely during normal query execution.
(It's now only called when, eg, we copy the ParamListInfo into a cursor
portal. copyParamList() or SerializeParamList() effectively instantiate
the virtual parameter array as a simple physical array without a
paramFetch hook, which is what we want in those cases.) This allows
reverting most of commit 6c82d8d1f, though I kept the cosmetic
code-consolidation aspects of that (eg the assign_simple_var function).

Performance testing shows this to be at worst a break-even change,
and it can provide wins ranging up to 20% in test cases involving
accesses to fields of "record" variables. The fact that values of
such variables can now be exposed to the planner might produce wins
in some situations, too, but I've not pursued that angle.

In passing, remove the "parent" pointer from the arguments to
ExecInitExprRec and related functions, instead storing that pointer in a
transient field in ExprState. The ParamListInfo pointer for a stand-alone
expression is handled the same way; we'd otherwise have had to add
yet another recursively-passed-down argument in expression compilation.

Discussion: https://postgr.es/m/32589.1513706441@sss.pgh.pa.us

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/6719b238e8f0ea83c39d2b1728c7670cdaa34e06

Modified Files
--------------
src/backend/commands/prepare.c | 3 +-
src/backend/executor/execCurrent.c | 9 +-
src/backend/executor/execExpr.c | 231 ++++++++++------
src/backend/executor/execExprInterp.c | 17 +-
src/backend/executor/functions.c | 3 +-
src/backend/executor/spi.c | 3 +-
src/backend/nodes/params.c | 64 ++---
src/backend/optimizer/util/clauses.c | 19 +-
src/backend/tcop/postgres.c | 6 +-
src/include/executor/execExpr.h | 23 +-
src/include/executor/executor.h | 1 +
src/include/nodes/execnodes.h | 21 +-
src/include/nodes/params.h | 84 ++++--
src/pl/plpgsql/src/pl_comp.c | 18 --
src/pl/plpgsql/src/pl_exec.c | 508 +++++++++++++++++++---------------
src/pl/plpgsql/src/plpgsql.h | 9 +-
16 files changed, 613 insertions(+), 406 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Robert Haas 2017-12-21 18:21:36 pgsql: Adjust assertion in GetCurrentCommandId.
Previous Message Alvaro Herrera 2017-12-21 17:43:14 pgsql: Get rid of copy_partition_key