diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/backend/executor/spi.c | 72 | ||||
| -rw-r--r-- | src/include/executor/spi.h | 3 | ||||
| -rw-r--r-- | src/pl/plpgsql/src/pl_exec.c | 42 | ||||
| -rw-r--r-- | src/test/regress/expected/plpgsql.out | 15 | ||||
| -rw-r--r-- | src/test/regress/sql/plpgsql.sql | 13 |
5 files changed, 126 insertions, 19 deletions
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 3da2d263023..de8d59a8cdc 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -1132,6 +1132,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, Snapshot snapshot; MemoryContext oldcontext; Portal portal; + ErrorContextCallback spierrcontext; /* * Check that the plan is something the Portal code will special-case as @@ -1182,6 +1183,15 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, plansource->query_string); /* + * Setup error traceback support for ereport(), in case GetCachedPlan + * throws an error. + */ + spierrcontext.callback = _SPI_error_callback; + spierrcontext.arg = (void *) plansource->query_string; + spierrcontext.previous = error_context_stack; + error_context_stack = &spierrcontext; + + /* * Note: for a saved plan, we mustn't have any failure occur between * GetCachedPlan and PortalDefineQuery; that would result in leaking our * plancache refcount. @@ -1191,6 +1201,9 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, cplan = GetCachedPlan(plansource, paramLI, false); stmt_list = cplan->stmt_list; + /* Pop the error context stack */ + error_context_stack = spierrcontext.previous; + if (!plan->saved) { /* @@ -1552,6 +1565,65 @@ SPI_result_code_string(int code) return buf; } +/* + * SPI_plan_get_plan_sources --- get a SPI plan's underlying list of + * CachedPlanSources. + * + * This is exported so that pl/pgsql can use it (this beats letting pl/pgsql + * look directly into the SPIPlan for itself). It's not documented in + * spi.sgml because we'd just as soon not have too many places using this. + */ +List * +SPI_plan_get_plan_sources(SPIPlanPtr plan) +{ + Assert(plan->magic == _SPI_PLAN_MAGIC); + return plan->plancache_list; +} + +/* + * SPI_plan_get_cached_plan --- get a SPI plan's generic CachedPlan, + * if the SPI plan contains exactly one CachedPlanSource. If not, + * return NULL. Caller is responsible for doing ReleaseCachedPlan(). + * + * This is exported so that pl/pgsql can use it (this beats letting pl/pgsql + * look directly into the SPIPlan for itself). It's not documented in + * spi.sgml because we'd just as soon not have too many places using this. + */ +CachedPlan * +SPI_plan_get_cached_plan(SPIPlanPtr plan) +{ + CachedPlanSource *plansource; + CachedPlan *cplan; + ErrorContextCallback spierrcontext; + + Assert(plan->magic == _SPI_PLAN_MAGIC); + + /* Can't support one-shot plans here */ + if (plan->oneshot) + return NULL; + + /* Must have exactly one CachedPlanSource */ + if (list_length(plan->plancache_list) != 1) + return NULL; + plansource = (CachedPlanSource *) linitial(plan->plancache_list); + + /* Setup error traceback support for ereport() */ + spierrcontext.callback = _SPI_error_callback; + spierrcontext.arg = (void *) plansource->query_string; + spierrcontext.previous = error_context_stack; + error_context_stack = &spierrcontext; + + /* Get the generic plan for the query */ + cplan = GetCachedPlan(plansource, NULL, plan->saved); + Assert(cplan == plansource->gplan); + + /* Pop the error context stack */ + error_context_stack = spierrcontext.previous; + + return cplan; +} + + /* =================== private functions =================== */ /* diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h index 2415940d547..d4f1272cd81 100644 --- a/src/include/executor/spi.h +++ b/src/include/executor/spi.h @@ -103,6 +103,9 @@ extern bool SPI_is_cursor_plan(SPIPlanPtr plan); extern bool SPI_plan_is_valid(SPIPlanPtr plan); extern const char *SPI_result_code_string(int code); +extern List *SPI_plan_get_plan_sources(SPIPlanPtr plan); +extern CachedPlan *SPI_plan_get_cached_plan(SPIPlanPtr plan); + extern HeapTuple SPI_copytuple(HeapTuple tuple); extern HeapTupleHeader SPI_returntuple(HeapTuple tuple, TupleDesc tupdesc); extern HeapTuple SPI_modifytuple(Relation rel, HeapTuple tuple, int natts, diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 0ecf651f765..591a432f542 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -22,7 +22,7 @@ #include "access/tupconvert.h" #include "catalog/pg_proc.h" #include "catalog/pg_type.h" -#include "executor/spi_priv.h" +#include "executor/spi.h" #include "funcapi.h" #include "miscadmin.h" #include "nodes/nodeFuncs.h" @@ -3174,7 +3174,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate, exec_prepare_plan(estate, expr, 0); stmt->mod_stmt = false; - foreach(l, expr->plan->plancache_list) + foreach(l, SPI_plan_get_plan_sources(expr->plan)) { CachedPlanSource *plansource = (CachedPlanSource *) lfirst(l); ListCell *l2; @@ -4954,10 +4954,11 @@ loop_exit: * * It is possible though unlikely for a simple expression to become non-simple * (consider for example redefining a trivial view). We must handle that for - * correctness; fortunately it's normally inexpensive to do GetCachedPlan on a - * simple expression. We do not consider the other direction (non-simple - * expression becoming simple) because we'll still give correct results if - * that happens, and it's unlikely to be worth the cycles to check. + * correctness; fortunately it's normally inexpensive to call + * SPI_plan_get_cached_plan for a simple expression. We do not consider the + * other direction (non-simple expression becoming simple) because we'll still + * give correct results if that happens, and it's unlikely to be worth the + * cycles to check. * * Note: if pass-by-reference, the result is in the eval_econtext's * temporary memory context. It will be freed when exec_eval_cleanup @@ -4973,7 +4974,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, { ExprContext *econtext = estate->eval_econtext; LocalTransactionId curlxid = MyProc->lxid; - CachedPlanSource *plansource; CachedPlan *cplan; ParamListInfo paramLI; PLpgSQL_expr *save_cur_expr; @@ -4993,16 +4993,16 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, /* * Revalidate cached plan, so that we will notice if it became stale. (We - * need to hold a refcount while using the plan, anyway.) Note that even - * if replanning occurs, the length of plancache_list can't change, since - * it is a property of the raw parsetree generated from the query text. + * need to hold a refcount while using the plan, anyway.) */ - Assert(list_length(expr->plan->plancache_list) == 1); - plansource = (CachedPlanSource *) linitial(expr->plan->plancache_list); + cplan = SPI_plan_get_cached_plan(expr->plan); - /* Get the generic plan for the query */ - cplan = GetCachedPlan(plansource, NULL, true); - Assert(cplan == plansource->gplan); + /* + * We can't get a failure here, because the number of CachedPlanSources in + * the SPI plan can't change from what exec_simple_check_plan saw; it's a + * property of the raw parsetree generated from the query text. + */ + Assert(cplan != NULL); if (cplan->generation != expr->expr_simple_generation) { @@ -5893,6 +5893,7 @@ exec_simple_check_node(Node *node) static void exec_simple_check_plan(PLpgSQL_expr *expr) { + List *plansources; CachedPlanSource *plansource; Query *query; CachedPlan *cplan; @@ -5908,9 +5909,10 @@ exec_simple_check_plan(PLpgSQL_expr *expr) /* * We can only test queries that resulted in exactly one CachedPlanSource */ - if (list_length(expr->plan->plancache_list) != 1) + plansources = SPI_plan_get_plan_sources(expr->plan); + if (list_length(plansources) != 1) return; - plansource = (CachedPlanSource *) linitial(expr->plan->plancache_list); + plansource = (CachedPlanSource *) linitial(plansources); /* * Do some checking on the analyzed-and-rewritten form of the query. These @@ -5968,8 +5970,10 @@ exec_simple_check_plan(PLpgSQL_expr *expr) */ /* Get the generic plan for the query */ - cplan = GetCachedPlan(plansource, NULL, true); - Assert(cplan == plansource->gplan); + cplan = SPI_plan_get_cached_plan(expr->plan); + + /* Can't fail, because we checked for a single CachedPlanSource above */ + Assert(cplan != NULL); /* Share the remaining work with recheck code path */ exec_simple_recheck_plan(expr, cplan); diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index dd1a8703fcc..fdd8c6b466e 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -4387,6 +4387,21 @@ select error2('public.stuffs'); rollback; drop function error2(p_name_table text); drop function error1(text); +-- Test for consistent reporting of error context +create function fail() returns int language plpgsql as $$ +begin + return 1/0; +end +$$; +select fail(); +ERROR: division by zero +CONTEXT: SQL statement "SELECT 1/0" +PL/pgSQL function fail() line 3 at RETURN +select fail(); +ERROR: division by zero +CONTEXT: SQL statement "SELECT 1/0" +PL/pgSQL function fail() line 3 at RETURN +drop function fail(); -- Test handling of string literals. set standard_conforming_strings = off; create or replace function strtest() returns text as $$ diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index fe507f4c152..017bd0b63f1 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -3537,6 +3537,19 @@ rollback; drop function error2(p_name_table text); drop function error1(text); +-- Test for consistent reporting of error context + +create function fail() returns int language plpgsql as $$ +begin + return 1/0; +end +$$; + +select fail(); +select fail(); + +drop function fail(); + -- Test handling of string literals. set standard_conforming_strings = off; |