On 23.08.2019 14:42, Pavel Stehule wrote:
In reality it is not IMMUTABLE function. On second hand, there are lot
of application that depends on this behave.
It is well know trick how to reduce estimation errors related to
JOINs. When immutable function has constant parameters, then it is
evaluated in planning time.
So sometimes was necessary to use
SELECT ... FROM tab WHERE foreign_key = immutable_function('constant
parameter')
instead JOIN.
It is ugly, but it is working perfectly. I think so until we will have
multi table statistics, this behave should be available in Postgres.
Sure, this function should not be used for functional indexes.
What about the following version of the patch?
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index a4697dc..c530c4d 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -6157,7 +6157,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
* updates made so far by our own function.
*/
oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate));
- if (!estate->readonly_func)
+ if (!estate->readonly_func && expr->expr_needs_snapshot)
{
CommandCounterIncrement();
PushActiveSnapshot(GetTransactionSnapshot());
@@ -6182,7 +6182,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
estate->paramLI->parserSetupArg = save_setup_arg;
- if (!estate->readonly_func)
+ if (!estate->readonly_func && expr->expr_needs_snapshot)
PopActiveSnapshot();
MemoryContextSwitchTo(oldcontext);
@@ -7978,6 +7978,43 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
}
/*
+ * expr_needs_snapshot --- check if expression contains calls of non-immutable functions or subqueries
+ */
+static bool
+expr_needs_snapshot(Node* expr, void* ctx)
+{
+ if (expr == NULL)
+ return false;
+ switch (nodeTag(expr))
+ {
+ case T_FuncExpr:
+ {
+ FuncExpr *func = (FuncExpr *)expr;
+ if (func_volatile(func->funcid) != PROVOLATILE_IMMUTABLE
+ || !IsCatalogNamespace(get_func_namespace(func->funcid)))
+ return true;
+ break;
+ }
+ case T_OpExpr:
+ {
+ OpExpr *op = (OpExpr *)expr;
+ if (func_volatile(op->opfuncid) != PROVOLATILE_IMMUTABLE
+ || !IsCatalogNamespace(get_func_namespace(op->opfuncid)))
+ return true;
+ break;
+ }
+ case T_Query:
+ case T_SubPlan:
+ case T_AlternativeSubPlan:
+ case T_SubLink:
+ return true;
+ default:
+ break;
+ }
+ return expression_tree_walker(expr, expr_needs_snapshot, ctx);
+}
+
+/*
* exec_save_simple_expr --- extract simple expression from CachedPlan
*/
static void
@@ -8046,6 +8083,7 @@ exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan)
* current transaction".
*/
expr->expr_simple_expr = tle_expr;
+ expr->expr_needs_snapshot = expr_needs_snapshot(tle_expr, NULL);
expr->expr_simple_generation = cplan->generation;
expr->expr_simple_state = NULL;
expr->expr_simple_in_use = false;
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index f66b2ba..454131f 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -243,6 +243,7 @@ typedef struct PLpgSQL_expr
*/
ExprState *expr_simple_state; /* eval tree for expr_simple_expr */
bool expr_simple_in_use; /* true if eval tree is active */
+ bool expr_needs_snapshot; /* true if simple expression calls non-immutable functions or performs subqueries */
LocalTransactionId expr_simple_lxid;
} PLpgSQL_expr;
