Avoid taking a new snapshot for an immutable simple expression in plpgsql.

We already used this optimization if the plpgsql function is read-only.
But it seems okay to do it even in a read-write function, if the
expression contains only immutable functions/operators.  There would
only be a change of behavior if an "immutable" called function depends
on seeing database updates made during the current plpgsql function.
That's enough of a violation of the promise of immutability that anyone
who complains won't have much of a case.

The benefits are significant --- for simple cases like

  while i < 10000000
  loop
    i := i + 1;
  end loop;

I see net performance improvements around 45%.  Of course, real-world
cases won't get that much faster, but it ought to be noticeable.
At the very least, this removes much of the performance penalty that
used to exist for forgetting to mark a plpgsql function non-volatile.

Konstantin Knizhnik, reviewed by Pavel Stehule, cosmetic changes by me

Discussion: https://postgr.es/m/ed9da20e-01aa-d04b-d085-e6c16b14b9d7@postgrespro.ru

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index fd799b724dae53557d71a2cd6c9b61a3cdf9e5bf..4a9ed83030a703f3abdf7573fa81cd0bc32d9b71 100644 (file)
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -6079,6 +6079,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
        LocalTransactionId curlxid = MyProc->lxid;
        CachedPlan *cplan;
        void       *save_setup_arg;
+       bool            need_snapshot;
        MemoryContext oldcontext;
 
        /*
@@ -6150,12 +6151,19 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
 
        /*
         * We have to do some of the things SPI_execute_plan would do, in
-        * particular advance the snapshot if we are in a non-read-only function.
-        * Without this, stable functions within the expression would fail to see
-        * updates made so far by our own function.
+        * particular push a new snapshot so that stable functions within the
+        * expression can see updates made so far by our own function.  However,
+        * we can skip doing that (and just invoke the expression with the same
+        * snapshot passed to our function) in some cases, which is useful because
+        * it's quite expensive relative to the cost of a simple expression.  We
+        * can skip it if the expression contains no stable or volatile functions;
+        * immutable functions shouldn't need to see our updates.  Also, if this
+        * is a read-only function, we haven't made any updates so again it's okay
+        * to skip.
         */
        oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate));
-       if (!estate->readonly_func)
+       need_snapshot = (expr->expr_simple_mutable && !estate->readonly_func);
+       if (need_snapshot)
        {
                CommandCounterIncrement();
                PushActiveSnapshot(GetTransactionSnapshot());
@@ -6180,7 +6188,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
 
        estate->paramLI->parserSetupArg = save_setup_arg;
 
-       if (!estate->readonly_func)
+       if (need_snapshot)
                PopActiveSnapshot();
 
        MemoryContextSwitchTo(oldcontext);
@@ -8051,6 +8059,8 @@ exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan)
        /* Also stash away the expression result type */
        expr->expr_simple_type = exprType((Node *) tle_expr);
        expr->expr_simple_typmod = exprTypmod((Node *) tle_expr);
+       /* We also want to remember if it is immutable or not */
+       expr->expr_simple_mutable = contain_mutable_functions((Node *) tle_expr);
 }
 
 /*

diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index f66b2baa70a9c36791a1433982d5a06e5a9483e0..6b40fcd1963ec2968a1e9ca98cefa39b25f8b34a 100644 (file)
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -234,6 +234,7 @@ typedef struct PLpgSQL_expr
        int                     expr_simple_generation; /* plancache generation we checked */
        Oid                     expr_simple_type;       /* result type Oid, if simple */
        int32           expr_simple_typmod; /* result typmod, if simple */
+       bool            expr_simple_mutable;    /* true if simple expr is mutable */
 
        /*
         * if expr is simple AND prepared in current transaction,