Обсуждение: Faster setup_param_list() in plpgsql
Given the rather hostile response I got to http://www.postgresql.org/message-id/4146.1425872254@sss.pgh.pa.us 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/22945.1424982755@sss.pgh.pa.us 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;
On 03/14/2015 06:04 PM, Tom Lane wrote: > Given the rather hostile response I got to > http://www.postgresql.org/message-id/4146.1425872254@sss.pgh.pa.us > 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/22945.1424982755@sss.pgh.pa.us > 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 ... > > I'm inclined to say go for it. I can recall cases in the past where we have found some significant piece of work to be necessary after feature freeze in order to enable a piece of work submitted before feature freeze to proceed. This sounds like a similar case. cheers andrew
I wrote:
> 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:
Here is a version that is rebased up to HEAD. Dunno if anyone wants
to re-review this, if not I'll go commit it.
regards, tom lane
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 0ff2086..05268e3 100644
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
*************** PLpgSQL_stmt_block *plpgsql_parse_result
*** 42,48 ****
static int datums_alloc;
int plpgsql_nDatums;
PLpgSQL_datum **plpgsql_Datums;
! static int datums_last = 0;
char *plpgsql_error_funcname;
bool plpgsql_DumpExecTree = false;
--- 42,48 ----
static int datums_alloc;
int plpgsql_nDatums;
PLpgSQL_datum **plpgsql_Datums;
! static int datums_last;
char *plpgsql_error_funcname;
bool plpgsql_DumpExecTree = false;
*************** 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
*** 2312,2317 ****
--- 2300,2321 ----
}
/* ----------
+ * 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)
*** 2329,2334 ****
--- 2333,2371 ----
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 79dd6a2..7d4001c 100644
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
*************** static int exec_for_query(PLpgSQL_execst
*** 216,221 ****
--- 216,223 ----
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
*** 242,249 ****
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);
--- 244,253 ----
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 *
*** 312,320 ****
{
PLpgSQL_var *var = (PLpgSQL_var *) estate.datums[n];
! var->value = fcinfo->arg[i];
! var->isnull = fcinfo->argnull[i];
! var->freeval = false;
/*
* Force any array-valued parameter to be stored in
--- 316,325 ----
{
PLpgSQL_var *var = (PLpgSQL_var *) estate.datums[n];
! assign_simple_var(&estate, var,
! fcinfo->arg[i],
! fcinfo->argnull[i],
! false);
/*
* Force any array-valued parameter to be stored in
*************** plpgsql_exec_function(PLpgSQL_function *
*** 336,344 ****
if (VARATT_IS_EXTERNAL_EXPANDED_RW(DatumGetPointer(var->value)))
{
/* take ownership of R/W object */
! var->value = TransferExpandedObject(var->value,
! CurrentMemoryContext);
! var->freeval = true;
}
else if (VARATT_IS_EXTERNAL_EXPANDED_RO(DatumGetPointer(var->value)))
{
--- 341,351 ----
if (VARATT_IS_EXTERNAL_EXPANDED_RW(DatumGetPointer(var->value)))
{
/* take ownership of R/W object */
! assign_simple_var(&estate, var,
! TransferExpandedObject(var->value,
! CurrentMemoryContext),
! false,
! true);
}
else if (VARATT_IS_EXTERNAL_EXPANDED_RO(DatumGetPointer(var->value)))
{
*************** plpgsql_exec_function(PLpgSQL_function *
*** 347,356 ****
else
{
/* flat array, so force to expanded form */
! var->value = expand_array(var->value,
! CurrentMemoryContext,
! NULL);
! var->freeval = true;
}
}
}
--- 354,365 ----
else
{
/* flat array, so force to expanded form */
! assign_simple_var(&estate, var,
! expand_array(var->value,
! CurrentMemoryContext,
! NULL),
! false,
! true);
}
}
}
*************** plpgsql_exec_trigger(PLpgSQL_function *f
*** 641,716 ****
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)
--- 650,718 ----
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
*** 730,747 ****
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");
--- 732,747 ----
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
*** 874,887 ****
* 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
--- 874,883 ----
* 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)
*** 1012,1021 ****
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;
}
--- 1008,1016 ----
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)
*** 1026,1036 ****
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;
}
--- 1021,1031 ----
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
*** 1114,1125 ****
{
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)
{
--- 1109,1119 ----
{
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
*** 1308,1316 ****
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
--- 1302,1310 ----
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
*** 1788,1798 ****
/* 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);
--- 1782,1788 ----
/* 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
*** 1801,1811 ****
/* 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)
--- 1791,1797 ----
/* 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
*** 2035,2042 ****
/*
* Assign current value to loop var
*/
! var->value = Int32GetDatum(loop_value);
! var->isnull = false;
/*
* Execute the statements
--- 2021,2027 ----
/*
* 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
*** 2228,2236 ****
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)
--- 2213,2221 ----
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
*** 2242,2252 ****
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
--- 2227,2241 ----
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
*** 2261,2271 ****
SPI_cursor_close(portal);
if (curname == NULL)
! {
! free_var(curvar);
! curvar->value = (Datum) 0;
! curvar->isnull = true;
! }
if (curname)
pfree(curname);
--- 2250,2256 ----
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 *
*** 3314,3319 ****
--- 3299,3305 ----
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_execsql(PLpgSQL_execstate *est
*** 3478,3484 ****
}
/*
! * Set up ParamListInfo (hook function and possibly data values)
*/
paramLI = setup_param_list(estate, expr);
--- 3464,3470 ----
}
/*
! * Set up ParamListInfo to pass to executor
*/
paramLI = setup_param_list(estate, expr);
*************** exec_stmt_open(PLpgSQL_execstate *estate
*** 3937,3943 ****
* 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;
}
--- 3923,3929 ----
* 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
*** 3991,3999 ****
}
/*
! * Set up ParamListInfo (hook function and possibly data values)
*/
! paramLI = setup_param_list(estate, query);
/*
* Open the cursor
--- 3977,3985 ----
}
/*
! * Set up short-lived ParamListInfo
*/
! paramLI = setup_unshared_param_list(estate, query);
/*
* Open the cursor
*************** exec_stmt_open(PLpgSQL_execstate *estate
*** 4009,4018 ****
* 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;
}
--- 3995,4006 ----
* 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
*** 4282,4300 ****
}
/*
! * Now free the old value, unless it's the same as the new
! * value (ie, we're doing "foo := foo"). Note that for
! * expanded objects, this test is necessary and cannot
! * reliably be made any earlier; we have to be looking at the
! * object's standard R/W pointer to be sure pointer equality
! * is meaningful.
*/
if (var->value != newvalue || var->isnull || isNull)
! free_var(var);
!
! var->value = newvalue;
! var->isnull = isNull;
! var->freeval = (!var->datatype->typbyval && !isNull);
break;
}
--- 4270,4285 ----
}
/*
! * Now free the old value, if any, and assign the new one. But
! * skip the assignment if old and new values are the same.
! * Note that for expanded objects, this test is necessary and
! * cannot reliably be made any earlier; we have to be looking
! * at the object's standard R/W pointer to be sure pointer
! * equality is meaningful.
*/
if (var->value != newvalue || var->isnull || isNull)
! assign_simple_var(estate, var, newvalue, isNull,
! (!var->datatype->typbyval && !isNull));
break;
}
*************** exec_assign_value(PLpgSQL_execstate *est
*** 4638,4644 ****
*
* 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: the returned Datum points right at the stored value in the case of
* pass-by-reference datatypes. Generally callers should take care not to
--- 4623,4630 ----
*
* 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: the returned Datum points right at the stored value in the case of
* pass-by-reference datatypes. Generally callers should take care not to
*************** exec_run_select(PLpgSQL_execstate *estat
*** 5082,5106 ****
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,
--- 5068,5099 ----
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 to pass to executor
+ */
+ paramLI = setup_param_list(estate, expr);
+
+ /*
* Execute the query
*/
rc = SPI_execute_plan_with_paramlist(expr->plan, paramLI,
*************** exec_eval_simple_expr(PLpgSQL_execstate
*** 5402,5408 ****
}
/*
! * Set up param list. For safety, save and restore
* estate->paramLI->parserSetupArg around our use of the param list.
*/
save_setup_arg = estate->paramLI->parserSetupArg;
--- 5395,5401 ----
}
/*
! * Set up ParamListInfo to pass to executor. For safety, save and restore
* estate->paramLI->parserSetupArg around our use of the param list.
*/
save_setup_arg = estate->paramLI->parserSetupArg;
*************** exec_eval_simple_expr(PLpgSQL_execstate
*** 5451,5473 ****
* 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)
--- 5444,5473 ----
* 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
*** 5488,5515 ****
*/
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. One of
! * them might be the variable the expression result will be assigned
! * to, in which case we can pass the variable's value as-is even if
! * it's a read-write expanded object; otherwise, convert read-write
! * pointers to read-only pointers for safety.
*/
dno = -1;
while ((dno = bms_next_member(expr->paramnos, dno)) >= 0)
--- 5488,5589 ----
*/
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
*** 5534,5546 ****
}
/*
- * 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.)
--- 5608,5613 ----
*************** plpgsql_param_fetch(ParamListInfo params
*** 5581,5600 ****
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);
/*
* If it's a read/write expanded datum, convert reference to read-only,
--- 5648,5697 ----
expr = (PLpgSQL_expr *) params->parserSetupArg;
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.
! */
! 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;
/*
* If it's a read/write expanded datum, convert reference to read-only,
*************** exec_set_found(PLpgSQL_execstate *estate
*** 6663,6670 ****
PLpgSQL_var *var;
var = (PLpgSQL_var *) (estate->datums[estate->found_varno]);
! var->value = BoolGetDatum(state);
! var->isnull = false;
}
/*
--- 6760,6766 ----
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
*** 6799,6812 ****
}
/*
! * 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)
{
if (DatumIsReadWriteExpandedObject(var->value,
--- 6895,6913 ----
}
/*
! * 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)
{
if (DatumIsReadWriteExpandedObject(var->value,
*************** free_var(PLpgSQL_var *var)
*** 6815,6834 ****
DeleteExpandedObject(var->value);
else
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;
}
/*
--- 6916,6944 ----
DeleteExpandedObject(var->value);
else
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 = MakeExpandedObjectReadOnly(newvalue,
+ isnull,
+ var->datatype->typlen);
+ 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 93c2504..0b78d95 100644
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
*************** typedef struct PLpgSQL_function
*** 752,759 ****
--- 752,763 ----
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
*** 796,801 ****
--- 800,806 ----
/* 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;