Given the rather hostile response I got to
http://www.postgresql.org/message-id/[email protected]
I was not planning to bring this topic up again until 9.6 development
starts. However, as I said in that thread, this work is getting done now
because of $dayjob deadlines, and I've realized that it would actually
make a lot of sense to apply it before my expanded-arrays patch that's
pending in the current commitfest. So I'm going to put on my flameproof
long johns and post it anyway. I will add it to the 2015-06 commitfest,
but I'd really rather deal with it now ...
What this patch does is to remove setup_param_list() overhead for the
common case of PLPGSQL_DTYPE_VAR variables (ie, any non-composite type).
It does that by the expedient of keeping the ParamExternData image of such
a variable valid at all times. That adds a few cycles to assignments to
these variables, but removes more cycles from each use of them. Unless
you believe that common plpgsql functions contain lots of dead stores,
this is a guaranteed win overall.
I'm seeing about 10% overall speedup (vs HEAD, with casserts off) for
realistic simple plpgsql logic, such as this test case:
create or replace function typicalspeed(n int) returns bigint as $$
declare res bigint := 0;
begin
for i in 1 .. n loop
res := res + i;
if i % 10 = 0 then res := res / 10; end if;
end loop;
return res;
end
$$ language plpgsql strict stable;
For functions with lots of variables (or even just lots of expressions,
since each one of those is a PLpgSQL_datum too), it's even more helpful.
I have found no cases where it makes things worse, at least to within
measurement error (run-to-run variability is a percent or two for me).
The reason I would like to apply this now rather than wait for 9.6
is that by making parameter management more explicit it removes the
need for the klugy changes in exec_eval_datum() that exist in
http://www.postgresql.org/message-id/[email protected]
Instead, we could leave exec_eval_datum() alone and substitute read-only
pointers only when manufacturing the parameter image of an expanded-object
variable. If we do it in the other order then we'll be making an API
change for exec_eval_datum() in 9.5 (assuming said patch gets in) and then
reverting it come 9.6.
So there you have it. Now, where'd I put those long johns ...
regards, tom lane
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 650cc48..9c201a7 100644
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
*************** static Node *make_datum_param(PLpgSQL_ex
*** 104,109 ****
--- 104,111 ----
static PLpgSQL_row *build_row_from_class(Oid classOid);
static PLpgSQL_row *build_row_from_vars(PLpgSQL_variable **vars, int numvars);
static PLpgSQL_type *build_datatype(HeapTuple typeTup, int32 typmod, Oid collation);
+ static void plpgsql_start_datums(void);
+ static void plpgsql_finish_datums(PLpgSQL_function *function);
static void compute_function_hashkey(FunctionCallInfo fcinfo,
Form_pg_proc procStruct,
PLpgSQL_func_hashkey *hashkey,
*************** do_compile(FunctionCallInfo fcinfo,
*** 371,383 ****
plpgsql_ns_init();
plpgsql_ns_push(NameStr(procStruct->proname));
plpgsql_DumpExecTree = false;
!
! datums_alloc = 128;
! plpgsql_nDatums = 0;
! /* This is short-lived, so needn't allocate in function's cxt */
! plpgsql_Datums = MemoryContextAlloc(compile_tmp_cxt,
! sizeof(PLpgSQL_datum *) * datums_alloc);
! datums_last = 0;
switch (function->fn_is_trigger)
{
--- 373,379 ----
plpgsql_ns_init();
plpgsql_ns_push(NameStr(procStruct->proname));
plpgsql_DumpExecTree = false;
! plpgsql_start_datums();
switch (function->fn_is_trigger)
{
*************** do_compile(FunctionCallInfo fcinfo,
*** 758,767 ****
function->fn_nargs = procStruct->pronargs;
for (i = 0; i < function->fn_nargs; i++)
function->fn_argvarnos[i] = in_arg_varnos[i];
! function->ndatums = plpgsql_nDatums;
! function->datums = palloc(sizeof(PLpgSQL_datum *) * plpgsql_nDatums);
! for (i = 0; i < plpgsql_nDatums; i++)
! function->datums[i] = plpgsql_Datums[i];
/* Debug dump for completed functions */
if (plpgsql_DumpExecTree)
--- 754,761 ----
function->fn_nargs = procStruct->pronargs;
for (i = 0; i < function->fn_nargs; i++)
function->fn_argvarnos[i] = in_arg_varnos[i];
!
! plpgsql_finish_datums(function);
/* Debug dump for completed functions */
if (plpgsql_DumpExecTree)
*************** plpgsql_compile_inline(char *proc_source
*** 804,810 ****
PLpgSQL_variable *var;
int parse_rc;
MemoryContext func_cxt;
- int i;
/*
* Setup the scanner input and error info. We assume that this function
--- 798,803 ----
*************** plpgsql_compile_inline(char *proc_source
*** 860,870 ****
plpgsql_ns_init();
plpgsql_ns_push(func_name);
plpgsql_DumpExecTree = false;
!
! datums_alloc = 128;
! plpgsql_nDatums = 0;
! plpgsql_Datums = palloc(sizeof(PLpgSQL_datum *) * datums_alloc);
! datums_last = 0;
/* Set up as though in a function returning VOID */
function->fn_rettype = VOIDOID;
--- 853,859 ----
plpgsql_ns_init();
plpgsql_ns_push(func_name);
plpgsql_DumpExecTree = false;
! plpgsql_start_datums();
/* Set up as though in a function returning VOID */
function->fn_rettype = VOIDOID;
*************** plpgsql_compile_inline(char *proc_source
*** 911,920 ****
* Complete the function's info
*/
function->fn_nargs = 0;
! function->ndatums = plpgsql_nDatums;
! function->datums = palloc(sizeof(PLpgSQL_datum *) * plpgsql_nDatums);
! for (i = 0; i < plpgsql_nDatums; i++)
! function->datums[i] = plpgsql_Datums[i];
/*
* Pop the error context stack
--- 900,907 ----
* Complete the function's info
*/
function->fn_nargs = 0;
!
! plpgsql_finish_datums(function);
/*
* Pop the error context stack
*************** plpgsql_build_record(const char *refname
*** 1965,1970 ****
--- 1952,1958 ----
rec->tup = NULL;
rec->tupdesc = NULL;
rec->freetup = false;
+ rec->freetupdesc = false;
plpgsql_adddatum((PLpgSQL_datum *) rec);
if (add2namespace)
plpgsql_ns_additem(PLPGSQL_NSTYPE_REC, rec->dno, rec->refname);
*************** plpgsql_parse_err_condition(char *condna
*** 2296,2301 ****
--- 2284,2305 ----
}
/* ----------
+ * plpgsql_start_datums Initialize datum list at compile startup.
+ * ----------
+ */
+ static void
+ plpgsql_start_datums(void)
+ {
+ datums_alloc = 128;
+ plpgsql_nDatums = 0;
+ /* This is short-lived, so needn't allocate in function's cxt */
+ plpgsql_Datums = MemoryContextAlloc(compile_tmp_cxt,
+ sizeof(PLpgSQL_datum *) * datums_alloc);
+ /* datums_last tracks what's been seen by plpgsql_add_initdatums() */
+ datums_last = 0;
+ }
+
+ /* ----------
* plpgsql_adddatum Add a variable, record or row
* to the compiler's datum list.
* ----------
*************** plpgsql_adddatum(PLpgSQL_datum *new)
*** 2313,2318 ****
--- 2317,2355 ----
plpgsql_Datums[plpgsql_nDatums++] = new;
}
+ /* ----------
+ * plpgsql_finish_datums Copy completed datum info into function struct.
+ *
+ * This is also responsible for building resettable_datums, a bitmapset
+ * of the dnos of all ROW, REC, and RECFIELD datums in the function.
+ * ----------
+ */
+ static void
+ plpgsql_finish_datums(PLpgSQL_function *function)
+ {
+ Bitmapset *resettable_datums = NULL;
+ int i;
+
+ function->ndatums = plpgsql_nDatums;
+ function->datums = palloc(sizeof(PLpgSQL_datum *) * plpgsql_nDatums);
+ for (i = 0; i < plpgsql_nDatums; i++)
+ {
+ function->datums[i] = plpgsql_Datums[i];
+ switch (function->datums[i]->dtype)
+ {
+ case PLPGSQL_DTYPE_ROW:
+ case PLPGSQL_DTYPE_REC:
+ case PLPGSQL_DTYPE_RECFIELD:
+ resettable_datums = bms_add_member(resettable_datums, i);
+ break;
+
+ default:
+ break;
+ }
+ }
+ function->resettable_datums = resettable_datums;
+ }
+
/* ----------
* plpgsql_add_initdatums Make an array of the datum numbers of
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index e332fa0..f455311 100644
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
*************** static int exec_for_query(PLpgSQL_execst
*** 211,216 ****
--- 211,218 ----
Portal portal, bool prefetch_ok);
static ParamListInfo setup_param_list(PLpgSQL_execstate *estate,
PLpgSQL_expr *expr);
+ static ParamListInfo setup_unshared_param_list(PLpgSQL_execstate *estate,
+ PLpgSQL_expr *expr);
static void plpgsql_param_fetch(ParamListInfo params, int paramid);
static void exec_move_row(PLpgSQL_execstate *estate,
PLpgSQL_rec *rec,
*************** static void exec_init_tuple_store(PLpgSQ
*** 237,244 ****
static void exec_set_found(PLpgSQL_execstate *estate, bool state);
static void plpgsql_create_econtext(PLpgSQL_execstate *estate);
static void plpgsql_destroy_econtext(PLpgSQL_execstate *estate);
! static void free_var(PLpgSQL_var *var);
! static void assign_text_var(PLpgSQL_var *var, const char *str);
static PreparedParamsData *exec_eval_using_params(PLpgSQL_execstate *estate,
List *params);
static void free_params_data(PreparedParamsData *ppd);
--- 239,248 ----
static void exec_set_found(PLpgSQL_execstate *estate, bool state);
static void plpgsql_create_econtext(PLpgSQL_execstate *estate);
static void plpgsql_destroy_econtext(PLpgSQL_execstate *estate);
! static void assign_simple_var(PLpgSQL_execstate *estate, PLpgSQL_var *var,
! Datum newvalue, bool isnull, bool freeable);
! static void assign_text_var(PLpgSQL_execstate *estate, PLpgSQL_var *var,
! const char *str);
static PreparedParamsData *exec_eval_using_params(PLpgSQL_execstate *estate,
List *params);
static void free_params_data(PreparedParamsData *ppd);
*************** plpgsql_exec_function(PLpgSQL_function *
*** 307,315 ****
{
PLpgSQL_var *var = (PLpgSQL_var *) estate.datums[n];
! var->value = fcinfo->arg[i];
! var->isnull = fcinfo->argnull[i];
! var->freeval = false;
}
break;
--- 311,320 ----
{
PLpgSQL_var *var = (PLpgSQL_var *) estate.datums[n];
! assign_simple_var(&estate, var,
! fcinfo->arg[i],
! fcinfo->argnull[i],
! false);
}
break;
*************** plpgsql_exec_trigger(PLpgSQL_function *f
*** 602,677 ****
var = (PLpgSQL_var *) (estate.datums[func->tg_op_varno]);
if (TRIGGER_FIRED_BY_INSERT(trigdata->tg_event))
! var->value = CStringGetTextDatum("INSERT");
else if (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
! var->value = CStringGetTextDatum("UPDATE");
else if (TRIGGER_FIRED_BY_DELETE(trigdata->tg_event))
! var->value = CStringGetTextDatum("DELETE");
else if (TRIGGER_FIRED_BY_TRUNCATE(trigdata->tg_event))
! var->value = CStringGetTextDatum("TRUNCATE");
else
elog(ERROR, "unrecognized trigger action: not INSERT, DELETE, UPDATE, or TRUNCATE");
- var->isnull = false;
- var->freeval = true;
var = (PLpgSQL_var *) (estate.datums[func->tg_name_varno]);
! var->value = DirectFunctionCall1(namein,
! CStringGetDatum(trigdata->tg_trigger->tgname));
! var->isnull = false;
! var->freeval = true;
var = (PLpgSQL_var *) (estate.datums[func->tg_when_varno]);
if (TRIGGER_FIRED_BEFORE(trigdata->tg_event))
! var->value = CStringGetTextDatum("BEFORE");
else if (TRIGGER_FIRED_AFTER(trigdata->tg_event))
! var->value = CStringGetTextDatum("AFTER");
else if (TRIGGER_FIRED_INSTEAD(trigdata->tg_event))
! var->value = CStringGetTextDatum("INSTEAD OF");
else
elog(ERROR, "unrecognized trigger execution time: not BEFORE, AFTER, or INSTEAD OF");
- var->isnull = false;
- var->freeval = true;
var = (PLpgSQL_var *) (estate.datums[func->tg_level_varno]);
if (TRIGGER_FIRED_FOR_ROW(trigdata->tg_event))
! var->value = CStringGetTextDatum("ROW");
else if (TRIGGER_FIRED_FOR_STATEMENT(trigdata->tg_event))
! var->value = CStringGetTextDatum("STATEMENT");
else
elog(ERROR, "unrecognized trigger event type: not ROW or STATEMENT");
- var->isnull = false;
- var->freeval = true;
var = (PLpgSQL_var *) (estate.datums[func->tg_relid_varno]);
! var->value = ObjectIdGetDatum(trigdata->tg_relation->rd_id);
! var->isnull = false;
! var->freeval = false;
var = (PLpgSQL_var *) (estate.datums[func->tg_relname_varno]);
! var->value = DirectFunctionCall1(namein,
! CStringGetDatum(RelationGetRelationName(trigdata->tg_relation)));
! var->isnull = false;
! var->freeval = true;
var = (PLpgSQL_var *) (estate.datums[func->tg_table_name_varno]);
! var->value = DirectFunctionCall1(namein,
! CStringGetDatum(RelationGetRelationName(trigdata->tg_relation)));
! var->isnull = false;
! var->freeval = true;
var = (PLpgSQL_var *) (estate.datums[func->tg_table_schema_varno]);
! var->value = DirectFunctionCall1(namein,
! CStringGetDatum(
! get_namespace_name(
RelationGetNamespace(
! trigdata->tg_relation))));
! var->isnull = false;
! var->freeval = true;
var = (PLpgSQL_var *) (estate.datums[func->tg_nargs_varno]);
! var->value = Int16GetDatum(trigdata->tg_trigger->tgnargs);
! var->isnull = false;
! var->freeval = false;
var = (PLpgSQL_var *) (estate.datums[func->tg_argv_varno]);
if (trigdata->tg_trigger->tgnargs > 0)
--- 607,675 ----
var = (PLpgSQL_var *) (estate.datums[func->tg_op_varno]);
if (TRIGGER_FIRED_BY_INSERT(trigdata->tg_event))
! assign_text_var(&estate, var, "INSERT");
else if (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
! assign_text_var(&estate, var, "UPDATE");
else if (TRIGGER_FIRED_BY_DELETE(trigdata->tg_event))
! assign_text_var(&estate, var, "DELETE");
else if (TRIGGER_FIRED_BY_TRUNCATE(trigdata->tg_event))
! assign_text_var(&estate, var, "TRUNCATE");
else
elog(ERROR, "unrecognized trigger action: not INSERT, DELETE, UPDATE, or TRUNCATE");
var = (PLpgSQL_var *) (estate.datums[func->tg_name_varno]);
! assign_simple_var(&estate, var,
! DirectFunctionCall1(namein,
! CStringGetDatum(trigdata->tg_trigger->tgname)),
! false, true);
var = (PLpgSQL_var *) (estate.datums[func->tg_when_varno]);
if (TRIGGER_FIRED_BEFORE(trigdata->tg_event))
! assign_text_var(&estate, var, "BEFORE");
else if (TRIGGER_FIRED_AFTER(trigdata->tg_event))
! assign_text_var(&estate, var, "AFTER");
else if (TRIGGER_FIRED_INSTEAD(trigdata->tg_event))
! assign_text_var(&estate, var, "INSTEAD OF");
else
elog(ERROR, "unrecognized trigger execution time: not BEFORE, AFTER, or INSTEAD OF");
var = (PLpgSQL_var *) (estate.datums[func->tg_level_varno]);
if (TRIGGER_FIRED_FOR_ROW(trigdata->tg_event))
! assign_text_var(&estate, var, "ROW");
else if (TRIGGER_FIRED_FOR_STATEMENT(trigdata->tg_event))
! assign_text_var(&estate, var, "STATEMENT");
else
elog(ERROR, "unrecognized trigger event type: not ROW or STATEMENT");
var = (PLpgSQL_var *) (estate.datums[func->tg_relid_varno]);
! assign_simple_var(&estate, var,
! ObjectIdGetDatum(trigdata->tg_relation->rd_id),
! false, false);
var = (PLpgSQL_var *) (estate.datums[func->tg_relname_varno]);
! assign_simple_var(&estate, var,
! DirectFunctionCall1(namein,
! CStringGetDatum(RelationGetRelationName(trigdata->tg_relation))),
! false, true);
var = (PLpgSQL_var *) (estate.datums[func->tg_table_name_varno]);
! assign_simple_var(&estate, var,
! DirectFunctionCall1(namein,
! CStringGetDatum(RelationGetRelationName(trigdata->tg_relation))),
! false, true);
var = (PLpgSQL_var *) (estate.datums[func->tg_table_schema_varno]);
! assign_simple_var(&estate, var,
! DirectFunctionCall1(namein,
! CStringGetDatum(get_namespace_name(
RelationGetNamespace(
! trigdata->tg_relation)))),
! false, true);
var = (PLpgSQL_var *) (estate.datums[func->tg_nargs_varno]);
! assign_simple_var(&estate, var,
! Int16GetDatum(trigdata->tg_trigger->tgnargs),
! false, false);
var = (PLpgSQL_var *) (estate.datums[func->tg_argv_varno]);
if (trigdata->tg_trigger->tgnargs > 0)
*************** plpgsql_exec_trigger(PLpgSQL_function *f
*** 691,708 ****
dims[0] = nelems;
lbs[0] = 0;
! var->value = PointerGetDatum(construct_md_array(elems, NULL,
! 1, dims, lbs,
! TEXTOID,
! -1, false, 'i'));
! var->isnull = false;
! var->freeval = true;
}
else
{
! var->value = (Datum) 0;
! var->isnull = true;
! var->freeval = false;
}
estate.err_text = gettext_noop("during function entry");
--- 689,704 ----
dims[0] = nelems;
lbs[0] = 0;
! assign_simple_var(&estate, var,
! PointerGetDatum(construct_md_array(elems, NULL,
! 1, dims, lbs,
! TEXTOID,
! -1, false, 'i')),
! false, true);
}
else
{
! assign_simple_var(&estate, var, (Datum) 0, true, false);
}
estate.err_text = gettext_noop("during function entry");
*************** plpgsql_exec_event_trigger(PLpgSQL_funct
*** 835,848 ****
* Assign the special tg_ variables
*/
var = (PLpgSQL_var *) (estate.datums[func->tg_event_varno]);
! var->value = CStringGetTextDatum(trigdata->event);
! var->isnull = false;
! var->freeval = true;
var = (PLpgSQL_var *) (estate.datums[func->tg_tag_varno]);
! var->value = CStringGetTextDatum(trigdata->tag);
! var->isnull = false;
! var->freeval = true;
/*
* Let the instrumentation plugin peek at this function
--- 831,840 ----
* Assign the special tg_ variables
*/
var = (PLpgSQL_var *) (estate.datums[func->tg_event_varno]);
! assign_text_var(&estate, var, trigdata->event);
var = (PLpgSQL_var *) (estate.datums[func->tg_tag_varno]);
! assign_text_var(&estate, var, trigdata->tag);
/*
* Let the instrumentation plugin peek at this function
*************** copy_plpgsql_datum(PLpgSQL_datum *datum)
*** 973,982 ****
PLpgSQL_var *new = palloc(sizeof(PLpgSQL_var));
memcpy(new, datum, sizeof(PLpgSQL_var));
! /* Ensure the value is null (possibly not needed?) */
! new->value = 0;
! new->isnull = true;
! new->freeval = false;
result = (PLpgSQL_datum *) new;
}
--- 965,973 ----
PLpgSQL_var *new = palloc(sizeof(PLpgSQL_var));
memcpy(new, datum, sizeof(PLpgSQL_var));
! /* should be preset to null/non-freeable */
! Assert(new->isnull);
! Assert(!new->freeval);
result = (PLpgSQL_datum *) new;
}
*************** copy_plpgsql_datum(PLpgSQL_datum *datum)
*** 987,997 ****
PLpgSQL_rec *new = palloc(sizeof(PLpgSQL_rec));
memcpy(new, datum, sizeof(PLpgSQL_rec));
! /* Ensure the value is null (possibly not needed?) */
! new->tup = NULL;
! new->tupdesc = NULL;
! new->freetup = false;
! new->freetupdesc = false;
result = (PLpgSQL_datum *) new;
}
--- 978,988 ----
PLpgSQL_rec *new = palloc(sizeof(PLpgSQL_rec));
memcpy(new, datum, sizeof(PLpgSQL_rec));
! /* should be preset to null/non-freeable */
! Assert(new->tup == NULL);
! Assert(new->tupdesc == NULL);
! Assert(!new->freetup);
! Assert(!new->freetupdesc);
result = (PLpgSQL_datum *) new;
}
*************** exec_stmt_block(PLpgSQL_execstate *estat
*** 1073,1084 ****
{
PLpgSQL_var *var = (PLpgSQL_var *) (estate->datums[n]);
! /* free any old value, in case re-entering block */
! free_var(var);
!
! /* Initially it contains a NULL */
! var->value = (Datum) 0;
! var->isnull = true;
if (var->default_val == NULL)
{
--- 1064,1074 ----
{
PLpgSQL_var *var = (PLpgSQL_var *) (estate->datums[n]);
! /*
! * Free any old value, in case re-entering block, and
! * initialize to NULL
! */
! assign_simple_var(estate, var, (Datum) 0, true, false);
if (var->default_val == NULL)
{
*************** exec_stmt_block(PLpgSQL_execstate *estat
*** 1267,1275 ****
errm_var = (PLpgSQL_var *)
estate->datums[block->exceptions->sqlerrm_varno];
! assign_text_var(state_var,
unpack_sql_state(edata->sqlerrcode));
! assign_text_var(errm_var, edata->message);
/*
* Also set up cur_error so the error data is accessible
--- 1257,1265 ----
errm_var = (PLpgSQL_var *)
estate->datums[block->exceptions->sqlerrm_varno];
! assign_text_var(estate, state_var,
unpack_sql_state(edata->sqlerrcode));
! assign_text_var(estate, errm_var, edata->message);
/*
* Also set up cur_error so the error data is accessible
*************** exec_stmt_case(PLpgSQL_execstate *estate
*** 1743,1753 ****
/* We can now discard any value we had for the temp variable */
if (t_var != NULL)
! {
! free_var(t_var);
! t_var->value = (Datum) 0;
! t_var->isnull = true;
! }
/* Evaluate the statement(s), and we're done */
return exec_stmts(estate, cwt->stmts);
--- 1733,1739 ----
/* We can now discard any value we had for the temp variable */
if (t_var != NULL)
! assign_simple_var(estate, t_var, (Datum) 0, true, false);
/* Evaluate the statement(s), and we're done */
return exec_stmts(estate, cwt->stmts);
*************** exec_stmt_case(PLpgSQL_execstate *estate
*** 1756,1766 ****
/* We can now discard any value we had for the temp variable */
if (t_var != NULL)
! {
! free_var(t_var);
! t_var->value = (Datum) 0;
! t_var->isnull = true;
! }
/* SQL2003 mandates this error if there was no ELSE clause */
if (!stmt->have_else)
--- 1742,1748 ----
/* We can now discard any value we had for the temp variable */
if (t_var != NULL)
! assign_simple_var(estate, t_var, (Datum) 0, true, false);
/* SQL2003 mandates this error if there was no ELSE clause */
if (!stmt->have_else)
*************** exec_stmt_fori(PLpgSQL_execstate *estate
*** 1990,1997 ****
/*
* Assign current value to loop var
*/
! var->value = Int32GetDatum(loop_value);
! var->isnull = false;
/*
* Execute the statements
--- 1972,1978 ----
/*
* Assign current value to loop var
*/
! assign_simple_var(estate, var, Int32GetDatum(loop_value), false, false);
/*
* Execute the statements
*************** exec_stmt_forc(PLpgSQL_execstate *estate
*** 2183,2191 ****
exec_prepare_plan(estate, query, curvar->cursor_options);
/*
! * Set up ParamListInfo (hook function and possibly data values)
*/
! paramLI = setup_param_list(estate, query);
/*
* Open the cursor (the paramlist will get copied into the portal)
--- 2164,2172 ----
exec_prepare_plan(estate, query, curvar->cursor_options);
/*
! * Set up short-lived ParamListInfo
*/
! paramLI = setup_unshared_param_list(estate, query);
/*
* Open the cursor (the paramlist will get copied into the portal)
*************** exec_stmt_forc(PLpgSQL_execstate *estate
*** 2197,2207 ****
elog(ERROR, "could not open cursor: %s",
SPI_result_code_string(SPI_result));
/*
* If cursor variable was NULL, store the generated portal name in it
*/
if (curname == NULL)
! assign_text_var(curvar, portal->name);
/*
* Execute the loop. We can't prefetch because the cursor is accessible
--- 2178,2192 ----
elog(ERROR, "could not open cursor: %s",
SPI_result_code_string(SPI_result));
+ /* don't need paramlist any more */
+ if (paramLI)
+ pfree(paramLI);
+
/*
* If cursor variable was NULL, store the generated portal name in it
*/
if (curname == NULL)
! assign_text_var(estate, curvar, portal->name);
/*
* Execute the loop. We can't prefetch because the cursor is accessible
*************** exec_stmt_forc(PLpgSQL_execstate *estate
*** 2216,2226 ****
SPI_cursor_close(portal);
if (curname == NULL)
! {
! free_var(curvar);
! curvar->value = (Datum) 0;
! curvar->isnull = true;
! }
if (curname)
pfree(curname);
--- 2201,2207 ----
SPI_cursor_close(portal);
if (curname == NULL)
! assign_simple_var(estate, curvar, (Datum) 0, true, false);
if (curname)
pfree(curname);
*************** plpgsql_estate_setup(PLpgSQL_execstate *
*** 3174,3179 ****
--- 3155,3161 ----
estate->paramLI->parserSetup = (ParserSetupHook) plpgsql_parser_setup;
estate->paramLI->parserSetupArg = NULL; /* filled during use */
estate->paramLI->numParams = estate->ndatums;
+ estate->params_dirty = false;
/* set up for use of appropriate simple-expression EState */
if (simple_eval_estate)
*************** exec_stmt_open(PLpgSQL_execstate *estate
*** 3790,3796 ****
* If cursor variable was NULL, store the generated portal name in it
*/
if (curname == NULL)
! assign_text_var(curvar, portal->name);
return PLPGSQL_RC_OK;
}
--- 3772,3778 ----
* If cursor variable was NULL, store the generated portal name in it
*/
if (curname == NULL)
! assign_text_var(estate, curvar, portal->name);
return PLPGSQL_RC_OK;
}
*************** exec_stmt_open(PLpgSQL_execstate *estate
*** 3844,3852 ****
}
/*
! * Set up ParamListInfo (hook function and possibly data values)
*/
! paramLI = setup_param_list(estate, query);
/*
* Open the cursor
--- 3826,3834 ----
}
/*
! * Set up short-lived ParamListInfo
*/
! paramLI = setup_unshared_param_list(estate, query);
/*
* Open the cursor
*************** exec_stmt_open(PLpgSQL_execstate *estate
*** 3862,3871 ****
* If cursor variable was NULL, store the generated portal name in it
*/
if (curname == NULL)
! assign_text_var(curvar, portal->name);
if (curname)
pfree(curname);
return PLPGSQL_RC_OK;
}
--- 3844,3855 ----
* If cursor variable was NULL, store the generated portal name in it
*/
if (curname == NULL)
! assign_text_var(estate, curvar, portal->name);
if (curname)
pfree(curname);
+ if (paramLI)
+ pfree(paramLI);
return PLPGSQL_RC_OK;
}
*************** exec_assign_value(PLpgSQL_execstate *est
*** 4098,4115 ****
var->datatype->typlen);
/*
! * Now free the old value. (We can't do this any earlier
! * because of the possibility that we are assigning the var's
! * old value to it, eg "foo := foo". We could optimize out
! * the assignment altogether in such cases, but it's too
! * infrequent to be worth testing for.)
*/
! free_var(var);
!
! var->value = newvalue;
! var->isnull = isNull;
! if (!var->datatype->typbyval && !isNull)
! var->freeval = true;
break;
}
--- 4082,4097 ----
var->datatype->typlen);
/*
! * Now free the old value, if any, and assign the new one.
! *
! * (We can't free the old value any earlier because of the
! * possibility that we are assigning the var's old value to
! * it, eg "foo := foo". We could optimize out the assignment
! * altogether in such cases, but it's too infrequent to be
! * worth testing for.)
*/
! assign_simple_var(estate, var, newvalue, isNull,
! (!var->datatype->typbyval && !isNull));
break;
}
*************** exec_assign_value(PLpgSQL_execstate *est
*** 4453,4459 ****
*
* The type oid, typmod, value in Datum format, and null flag are returned.
*
! * At present this doesn't handle PLpgSQL_expr or PLpgSQL_arrayelem datums.
*
* NOTE: caller must not modify the returned value, since it points right
* at the stored value in the case of pass-by-reference datatypes. In some
--- 4435,4442 ----
*
* The type oid, typmod, value in Datum format, and null flag are returned.
*
! * At present this doesn't handle PLpgSQL_expr or PLpgSQL_arrayelem datums;
! * that's not needed because we never pass references to such datums to SPI.
*
* NOTE: caller must not modify the returned value, since it points right
* at the stored value in the case of pass-by-reference datatypes. In some
*************** exec_run_select(PLpgSQL_execstate *estat
*** 4893,4917 ****
exec_prepare_plan(estate, expr, 0);
/*
- * Set up ParamListInfo (hook function and possibly data values)
- */
- paramLI = setup_param_list(estate, expr);
-
- /*
* If a portal was requested, put the query into the portal
*/
if (portalP != NULL)
{
*portalP = SPI_cursor_open_with_paramlist(NULL, expr->plan,
paramLI,
estate->readonly_func);
if (*portalP == NULL)
elog(ERROR, "could not open implicit cursor for query \"%s\": %s",
expr->query, SPI_result_code_string(SPI_result));
return SPI_OK_CURSOR;
}
/*
* Execute the query
*/
rc = SPI_execute_plan_with_paramlist(expr->plan, paramLI,
--- 4876,4907 ----
exec_prepare_plan(estate, expr, 0);
/*
* If a portal was requested, put the query into the portal
*/
if (portalP != NULL)
{
+ /*
+ * Set up short-lived ParamListInfo
+ */
+ paramLI = setup_unshared_param_list(estate, expr);
+
*portalP = SPI_cursor_open_with_paramlist(NULL, expr->plan,
paramLI,
estate->readonly_func);
if (*portalP == NULL)
elog(ERROR, "could not open implicit cursor for query \"%s\": %s",
expr->query, SPI_result_code_string(SPI_result));
+ if (paramLI)
+ pfree(paramLI);
return SPI_OK_CURSOR;
}
/*
+ * Set up ParamListInfo (hook function and possibly data values)
+ */
+ paramLI = setup_param_list(estate, expr);
+
+ /*
* Execute the query
*/
rc = SPI_execute_plan_with_paramlist(expr->plan, paramLI,
*************** exec_eval_simple_expr(PLpgSQL_execstate
*** 5259,5281 ****
* Create a ParamListInfo to pass to SPI
*
* We share a single ParamListInfo array across all SPI calls made from this
! * estate. This is generally OK since any given slot in the array would
! * need to contain the same current datum value no matter which query or
! * expression we're evaluating. However, paramLI->parserSetupArg points to
! * the specific PLpgSQL_expr being evaluated. This is not an issue for
! * statement-level callers, but lower-level callers should save and restore
! * estate->paramLI->parserSetupArg just in case there's an active evaluation
! * at an outer call level.
*
! * We fill in the values for any expression parameters that are plain
! * PLpgSQL_var datums; these are cheap and safe to evaluate, and by setting
! * them with PARAM_FLAG_CONST flags, we allow the planner to use those values
! * in custom plans. However, parameters that are not plain PLpgSQL_vars
! * should not be evaluated here, because they could throw errors (for example
! * "no such record field") and we do not want that to happen in a part of
! * the expression that might never be evaluated at runtime. To handle those
! * parameters, we set up a paramFetch hook for the executor to call when it
! * wants a not-presupplied value.
*/
static ParamListInfo
setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
--- 5249,5278 ----
* Create a ParamListInfo to pass to SPI
*
* We share a single ParamListInfo array across all SPI calls made from this
! * estate, except calls creating cursors, which use setup_unshared_param_list
! * (see its comments for reasons why). A shared array is generally OK since
! * any given slot in the array would need to contain the same current datum
! * value no matter which query or expression we're evaluating. However,
! * paramLI->parserSetupArg points to the specific PLpgSQL_expr being
! * evaluated. This is not an issue for statement-level callers, but
! * lower-level callers must save and restore estate->paramLI->parserSetupArg
! * just in case there's an active evaluation at an outer call level.
*
! * The general plan for passing parameters to SPI is that plain VAR datums
! * always have valid images in the shared param list. This is ensured by
! * assign_simple_var(), which also marks those params as PARAM_FLAG_CONST,
! * allowing the planner to use those values in custom plans. However, non-VAR
! * datums cannot conveniently be managed that way. For one thing, they could
! * throw errors (for example "no such record field") and we do not want that
! * to happen in a part of the expression that might never be evaluated at
! * runtime. For another thing, exec_eval_datum() may return short-lived
! * values stored in the estate's short-term memory context, which will not
! * necessarily survive to the next SPI operation. And for a third thing, ROW
! * and RECFIELD datums' values depend on other datums, and we don't have a
! * cheap way to track that. Therefore, param slots for non-VAR datum types
! * are always reset here and then filled on-demand by plpgsql_param_fetch().
! * We can save a few cycles by not bothering with the reset loop unless at
! * least one such param has actually been filled by plpgsql_param_fetch().
*/
static ParamListInfo
setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
*************** setup_param_list(PLpgSQL_execstate *esta
*** 5296,5318 ****
*/
if (expr->paramnos)
{
! int dno;
!
! /* Use the common ParamListInfo for all evals in this estate */
paramLI = estate->paramLI;
/*
! * Reset all entries to "invalid". It's pretty annoying to have to do
! * this, but we don't currently track enough information to know which
! * old entries might be obsolete. (There are a couple of nontrivial
! * issues that would have to be dealt with in order to do better here.
! * First, ROW and RECFIELD datums depend on other datums, and second,
! * exec_eval_datum() will return short-lived palloc'd values for ROW
! * and REC datums.)
*/
! MemSet(paramLI->params, 0, estate->ndatums * sizeof(ParamExternData));
! /* Instantiate values for "safe" parameters of the expression */
dno = -1;
while ((dno = bms_next_member(expr->paramnos, dno)) >= 0)
{
--- 5293,5395 ----
*/
if (expr->paramnos)
{
! /* Use the common ParamListInfo */
paramLI = estate->paramLI;
/*
! * If any resettable parameters have been passed to the executor since
! * last time, we need to reset those param slots to "invalid", for the
! * reasons mentioned in the comment above.
*/
! if (estate->params_dirty)
! {
! Bitmapset *resettable_datums = estate->func->resettable_datums;
! int dno = -1;
! while ((dno = bms_next_member(resettable_datums, dno)) >= 0)
! {
! ParamExternData *prm = ¶mLI->params[dno];
!
! prm->ptype = InvalidOid;
! }
! estate->params_dirty = false;
! }
!
! /*
! * Set up link to active expr where the hook functions can find it.
! * Callers must save and restore parserSetupArg if there is any chance
! * that they are interrupting an active use of parameters.
! */
! paramLI->parserSetupArg = (void *) expr;
!
! /*
! * Also make sure this is set before parser hooks need it. There is
! * no need to save and restore, since the value is always correct once
! * set. (Should be set already, but let's be sure.)
! */
! expr->func = estate->func;
! }
! else
! {
! /*
! * Expression requires no parameters. Be sure we represent this case
! * as a NULL ParamListInfo, so that plancache.c knows there is no
! * point in a custom plan.
! */
! paramLI = NULL;
! }
! return paramLI;
! }
!
! /*
! * Create an unshared, short-lived ParamListInfo to pass to SPI
! *
! * When creating a cursor, we do not use the shared ParamListInfo array
! * but create a short-lived one that will contain only params actually
! * referenced by the query. The reason for this is that copyParamList() will
! * be used to copy the parameters into cursor-lifespan storage, and we don't
! * want it to copy anything that's not used by the specific cursor; that
! * could result in uselessly copying some large values.
! *
! * Caller should pfree the result after use, if it's not NULL.
! */
! static ParamListInfo
! setup_unshared_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
! {
! ParamListInfo paramLI;
!
! /*
! * We must have created the SPIPlan already (hence, query text has been
! * parsed/analyzed at least once); else we cannot rely on expr->paramnos.
! */
! Assert(expr->plan != NULL);
!
! /*
! * We only need a ParamListInfo if the expression has parameters. In
! * principle we should test with bms_is_empty(), but we use a not-null
! * test because it's faster. In current usage bits are never removed from
! * expr->paramnos, only added, so this test is correct anyway.
! */
! if (expr->paramnos)
! {
! int dno;
!
! /* initialize ParamListInfo with one entry per datum, all invalid */
! paramLI = (ParamListInfo)
! palloc0(offsetof(ParamListInfoData, params) +
! estate->ndatums * sizeof(ParamExternData));
! paramLI->paramFetch = plpgsql_param_fetch;
! paramLI->paramFetchArg = (void *) estate;
! paramLI->parserSetup = (ParserSetupHook) plpgsql_parser_setup;
! paramLI->parserSetupArg = (void *) expr;
! paramLI->numParams = estate->ndatums;
!
! /*
! * Instantiate values for "safe" parameters of the expression. We
! * could skip this and leave them to be filled by plpgsql_param_fetch;
! * but then the values would not be available for query planning,
! * since the planner doesn't call the paramFetch hook.
! */
dno = -1;
while ((dno = bms_next_member(expr->paramnos, dno)) >= 0)
{
*************** setup_param_list(PLpgSQL_execstate *esta
*** 5331,5343 ****
}
/*
- * Set up link to active expr where the hook functions can find it.
- * Callers must save and restore parserSetupArg if there is any chance
- * that they are interrupting an active use of parameters.
- */
- paramLI->parserSetupArg = (void *) expr;
-
- /*
* Also make sure this is set before parser hooks need it. There is
* no need to save and restore, since the value is always correct once
* set. (Should be set already, but let's be sure.)
--- 5408,5413 ----
*************** plpgsql_param_fetch(ParamListInfo params
*** 5375,5397 ****
/* fetch back the hook data */
estate = (PLpgSQL_execstate *) params->paramFetchArg;
- expr = (PLpgSQL_expr *) params->parserSetupArg;
Assert(params->numParams == estate->ndatums);
! /*
! * Do nothing if asked for a value that's not supposed to be used by this
! * SQL expression. This avoids unwanted evaluations when functions such
! * as copyParamList try to materialize all the values.
! */
! if (!bms_is_member(dno, expr->paramnos))
! return;
/* OK, evaluate the value and store into the appropriate paramlist slot */
- datum = estate->datums[dno];
prm = ¶ms->params[dno];
exec_eval_datum(estate, datum,
&prm->ptype, &prmtypmod,
&prm->value, &prm->isnull);
}
--- 5445,5497 ----
/* fetch back the hook data */
estate = (PLpgSQL_execstate *) params->paramFetchArg;
Assert(params->numParams == estate->ndatums);
! /* now we can access the target datum */
! datum = estate->datums[dno];
!
! /* need to behave slightly differently for shared and unshared arrays */
! if (params != estate->paramLI)
! {
! /*
! * We're being called, presumably from copyParamList(), for cursor
! * parameters. Since copyParamList() will try to materialize every
! * single parameter slot, it's important to do nothing when asked for
! * a datum that's not supposed to be used by this SQL expression.
! * Otherwise we risk failures in exec_eval_datum(), not to mention
! * possibly copying a lot more data than the cursor actually uses.
! */
! expr = (PLpgSQL_expr *) params->parserSetupArg;
! if (!bms_is_member(dno, expr->paramnos))
! return;
! }
! else
! {
! /*
! * Normal evaluation cases. We don't need to sanity-check dno, but we
! * do need to mark the shared params array dirty if we're about to
! * evaluate a resettable datum.
! */
! switch (datum->dtype)
! {
! case PLPGSQL_DTYPE_ROW:
! case PLPGSQL_DTYPE_REC:
! case PLPGSQL_DTYPE_RECFIELD:
! estate->params_dirty = true;
! break;
!
! default:
! break;
! }
! }
/* OK, evaluate the value and store into the appropriate paramlist slot */
prm = ¶ms->params[dno];
exec_eval_datum(estate, datum,
&prm->ptype, &prmtypmod,
&prm->value, &prm->isnull);
+ /* We can always mark params as "const" for executor's purposes */
+ prm->pflags = PARAM_FLAG_CONST;
}
*************** exec_set_found(PLpgSQL_execstate *estate
*** 6344,6351 ****
PLpgSQL_var *var;
var = (PLpgSQL_var *) (estate->datums[estate->found_varno]);
! var->value = BoolGetDatum(state);
! var->isnull = false;
}
/*
--- 6444,6450 ----
PLpgSQL_var *var;
var = (PLpgSQL_var *) (estate->datums[estate->found_varno]);
! assign_simple_var(estate, var, BoolGetDatum(state), false, false);
}
/*
*************** plpgsql_subxact_cb(SubXactEvent event, S
*** 6480,6510 ****
}
/*
! * free_var --- pfree any pass-by-reference value of the variable.
*
! * This should always be followed by some assignment to var->value,
! * as it leaves a dangling pointer.
*/
static void
! free_var(PLpgSQL_var *var)
{
if (var->freeval)
- {
pfree(DatumGetPointer(var->value));
! var->freeval = false;
! }
}
/*
* free old value of a text variable and assign new value from C string
*/
static void
! assign_text_var(PLpgSQL_var *var, const char *str)
{
! free_var(var);
! var->value = CStringGetTextDatum(str);
! var->isnull = false;
! var->freeval = true;
}
/*
--- 6579,6619 ----
}
/*
! * assign_simple_var --- assign a new value to any VAR datum.
*
! * This should be the only mechanism for assignment to simple variables,
! * lest we forget to update the paramLI image.
*/
static void
! assign_simple_var(PLpgSQL_execstate *estate, PLpgSQL_var *var,
! Datum newvalue, bool isnull, bool freeable)
{
+ ParamExternData *prm;
+
+ Assert(var->dtype == PLPGSQL_DTYPE_VAR);
+ /* Free the old value if needed */
if (var->freeval)
pfree(DatumGetPointer(var->value));
! /* Assign new value to datum */
! var->value = newvalue;
! var->isnull = isnull;
! var->freeval = freeable;
! /* And update the image in the common parameter list */
! prm = &estate->paramLI->params[var->dno];
! prm->value = newvalue;
! prm->isnull = isnull;
! /* these might be set already, but let's be sure */
! prm->pflags = PARAM_FLAG_CONST;
! prm->ptype = var->datatype->typoid;
}
/*
* free old value of a text variable and assign new value from C string
*/
static void
! assign_text_var(PLpgSQL_execstate *estate, PLpgSQL_var *var, const char *str)
{
! assign_simple_var(estate, var, CStringGetTextDatum(str), false, true);
}
/*
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index 66d4da6..dfe3b34 100644
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
*************** typedef struct PLpgSQL_function
*** 742,749 ****
--- 742,753 ----
int extra_warnings;
int extra_errors;
+ /* the datums representing the function's local variables */
int ndatums;
PLpgSQL_datum **datums;
+ Bitmapset *resettable_datums; /* dnos of non-simple vars */
+
+ /* function body parsetree */
PLpgSQL_stmt_block *action;
/* table for performing casts needed in this function */
*************** typedef struct PLpgSQL_execstate
*** 786,791 ****
--- 790,796 ----
/* we pass datums[i] to the executor, when needed, in paramLI->params[i] */
ParamListInfo paramLI;
+ bool params_dirty; /* T if any resettable datum has been passed */
/* EState to use for "simple" expression evaluation */
EState *simple_eval_estate;
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers