diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b7c8d663fc..b292948853 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -16539,9 +16539,10 @@ CloneRowTriggersToPartition(Relation parent, Relation partition) * Internal triggers require careful examination. Ideally, we don't * clone them. * - * However, if our parent is a partitioned relation, there might be - * internal triggers that need cloning. In that case, we must skip - * clone it if the trigger on parent depends on another trigger. + * However, if our parent is a partition itself, there might be + * internal triggers that need cloning. For example, triggers on the + * parent that were in turn cloned from its own parent are marked + * internal, which too must be cloned to the partition. * * Note we dare not verify that the other trigger belongs to an * ancestor relation of our parent, because that creates deadlock diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 5acf604f63..72f156726f 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -319,8 +319,7 @@ static void exec_eval_cleanup(PLpgSQL_execstate *estate); static void exec_prepare_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr, int cursorOptions, bool keepplan); -static void exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr); -static void exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan); +static void exec_check_simple_expr(PLpgSQL_execstate *estate, PLpgSQL_expr *expr); static void exec_check_rw_parameter(PLpgSQL_expr *expr, int target_dno); static bool contains_target_param(Node *node, int *target_dno); static bool exec_eval_simple_expr(PLpgSQL_execstate *estate, @@ -4062,7 +4061,7 @@ exec_prepare_plan(PLpgSQL_execstate *estate, expr->plan = plan; /* Check to see if it's a simple expression */ - exec_simple_check_plan(estate, expr); + exec_check_simple_expr(estate, expr); /* * Mark expression as not using a read-write param. exec_assign_value has @@ -5758,9 +5757,10 @@ exec_eval_expr(PLpgSQL_execstate *estate, Form_pg_attribute attr; /* - * If first time through, create a plan for this expression. + * Create a plan for this expression, if first time through or the + * existing plan is no longer valid. */ - if (expr->plan == NULL) + if (expr->plan == NULL || !SPI_plan_is_valid(expr->plan)) exec_prepare_plan(estate, expr, CURSOR_OPT_PARALLEL_OK, true); /* @@ -6076,7 +6076,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, { ExprContext *econtext = estate->eval_econtext; LocalTransactionId curlxid = MyProc->lxid; - CachedPlan *cplan; void *save_setup_arg; bool need_snapshot; MemoryContext oldcontext; @@ -6093,30 +6092,15 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, if (expr->expr_simple_in_use && expr->expr_simple_lxid == curlxid) return false; - /* - * Revalidate cached plan, so that we will notice if it became stale. (We - * need to hold a refcount while using the plan, anyway.) If replanning - * is needed, do that work in the eval_mcontext. - */ - oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate)); - cplan = SPI_plan_get_cached_plan(expr->plan); - MemoryContextSwitchTo(oldcontext); + /* Simple expressions aren't planned, so generation wouldn't change. */ + Assert(expr->expr_simple_generation == 0); /* - * 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. + * better recheck r/w safety, as it could change due to inlining + * XXX - should no longer occur, because planning is not done? */ - Assert(cplan != NULL); - - /* If it got replanned, update our copy of the simple expression */ - if (cplan->generation != expr->expr_simple_generation) - { - exec_save_simple_expr(expr, cplan); - /* better recheck r/w safety, as it could change due to inlining */ - if (expr->rwparam >= 0) - exec_check_rw_parameter(expr, expr->rwparam); - } + if (expr->rwparam >= 0) + exec_check_rw_parameter(expr, expr->rwparam); /* * Pass back previously-determined result type. @@ -6192,11 +6176,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, MemoryContextSwitchTo(oldcontext); - /* - * Now we can release our refcount on the cached plan. - */ - ReleaseCachedPlan(cplan, true); - /* * That's it. */ @@ -7890,19 +7869,20 @@ get_cast_hashentry(PLpgSQL_execstate *estate, /* ---------- - * exec_simple_check_plan - Check if a plan is simple enough to + * exec_check_simple_expr - Check if the expression is simple enough to * be evaluated by ExecEvalExpr() instead * of SPI. + * + * If it is, set expr->expr_simple_expr. * ---------- */ static void -exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) +exec_check_simple_expr(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) { List *plansources; CachedPlanSource *plansource; Query *query; - CachedPlan *cplan; - MemoryContext oldcontext; + Expr *tle_expr; /* * Initialize to "not simple". @@ -7972,95 +7952,12 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) return; /* - * OK, we can treat it as a simple plan. - * - * Get the generic plan for the query. If replanning is needed, do that - * work in the eval_mcontext. - */ - oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate)); - cplan = SPI_plan_get_cached_plan(expr->plan); - MemoryContextSwitchTo(oldcontext); - - /* Can't fail, because we checked for a single CachedPlanSource above */ - Assert(cplan != NULL); - - /* Share the remaining work with replan code path */ - exec_save_simple_expr(expr, cplan); - - /* Release our plan refcount */ - ReleaseCachedPlan(cplan, true); -} - -/* - * exec_save_simple_expr --- extract simple expression from CachedPlan - */ -static void -exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan) -{ - PlannedStmt *stmt; - Plan *plan; - Expr *tle_expr; - - /* - * Given the checks that exec_simple_check_plan did, none of the Asserts - * here should ever fail. - */ - - /* Extract the single PlannedStmt */ - Assert(list_length(cplan->stmt_list) == 1); - stmt = linitial_node(PlannedStmt, cplan->stmt_list); - Assert(stmt->commandType == CMD_SELECT); - - /* - * Ordinarily, the plan node should be a simple Result. However, if - * force_parallel_mode is on, the planner might've stuck a Gather node - * atop that. The simplest way to deal with this is to look through the - * Gather node. The Gather node's tlist would normally contain a Var - * referencing the child node's output, but it could also be a Param, or - * it could be a Const that setrefs.c copied as-is. - */ - plan = stmt->planTree; - for (;;) - { - /* Extract the single tlist expression */ - Assert(list_length(plan->targetlist) == 1); - tle_expr = castNode(TargetEntry, linitial(plan->targetlist))->expr; - - if (IsA(plan, Result)) - { - Assert(plan->lefttree == NULL && - plan->righttree == NULL && - plan->initPlan == NULL && - plan->qual == NULL && - ((Result *) plan)->resconstantqual == NULL); - break; - } - else if (IsA(plan, Gather)) - { - Assert(plan->lefttree != NULL && - plan->righttree == NULL && - plan->initPlan == NULL && - plan->qual == NULL); - /* If setrefs.c copied up a Const, no need to look further */ - if (IsA(tle_expr, Const)) - break; - /* Otherwise, it had better be a Param or an outer Var */ - Assert(IsA(tle_expr, Param) ||(IsA(tle_expr, Var) && - ((Var *) tle_expr)->varno == OUTER_VAR)); - /* Descend to the child node */ - plan = plan->lefttree; - } - else - elog(ERROR, "unexpected plan node type: %d", - (int) nodeTag(plan)); - } - - /* - * Save the simple expression, and initialize state to "not valid in - * current transaction". + * We have a simple expression. Save it and initialize state to "not + * valid in current transaction". */ + tle_expr = castNode(TargetEntry, linitial(query->targetList))->expr; expr->expr_simple_expr = tle_expr; - expr->expr_simple_generation = cplan->generation; + expr->expr_simple_generation = plansource->generation; expr->expr_simple_state = NULL; expr->expr_simple_in_use = false; expr->expr_simple_lxid = InvalidLocalTransactionId; diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index cd2c79f4d5..3abbc7bc57 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -4289,12 +4289,10 @@ end $$; select fail(); ERROR: division by zero -CONTEXT: SQL statement "SELECT 1/0" -PL/pgSQL function fail() line 3 at RETURN +CONTEXT: 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 +CONTEXT: PL/pgSQL function fail() line 3 at RETURN drop function fail(); -- Test handling of string literals. set standard_conforming_strings = off;