Re: [HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1

[Aleksander Alekseev]

The following review has been posted through the commitfest application:
make installcheck-world:  not tested
Implements feature:   not tested
Spec compliant:   not tested
Documentation:not tested

Hi Marina,

I'm sorry to inform you that the v5 path set become a little outdated:

```
$ git apply v5-0002-Precalculate-stable-functions-planning-and-execut.patch
error: patch failed: src/pl/plpgsql/src/pl_exec.c:6471
error: src/pl/plpgsql/src/pl_exec.c: patch does not apply
```

If it's not too much trouble could you please fix the conflicts with the 
current master branch?

The new status of this patch is: Waiting on Author

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1

[Marina Polyakova]

Like for the previous patches it seems that there is
no obvious performance degradation too on regular queries (according 
to

pgbench).


pgbench probably isn't a very good test for this sort of thing - it
only issues very short-running queries where the cost of evaluating
expressions is a relatively small part of the total cost.  Even if
things get worse, I'm not sure if you'd see it.


If there's a mistake, for example, more than 1 try to replace cached 
expressions in the whole query tree, results of "in buffer test" or 
"mostly cache test" can different a little..



I'm not sure exactly
how you could construct a test case that could be harmed by this patch
- I guess you'd want to initialize lots of CacheExprs but never make
use of the caching usefully?


As I mentioned in the first letter about this feature it will be useful 
for such text search queries [1]:


SELECT COUNT(*) FROM messages WHERE body_tsvector @@ 
to_tsquery('postgres');


And I'm not sure that it is logical to precalculate stable and immutable 
functions themselves, but not to precalculate expressions that behave 
like stable/immutable functions; precalculate some types of operators 
and not to precalculate others (ScalarArrayOpExpr, RowCompareExpr). My 
patch solves the problem that not all nodes are simplified in 
eval_const_expressions_mutator (for example, ScalarArrayOpExpr) and 
consts of other types now behave more like ordinary consts (for example, 
composite types, coerce expressions, ConvertRowtypeExpr).



It could also be useful to test things like TPC-H to see if you get an
improvement.


I saw the examples of queries in TPC-H tests. If I'm not wrong they are 
not the target tests for this functionality (nothing will be 
precalculated). But it's a good idea to check that there's no a 
performance degradation on them too.


[1] 
https://www.postgresql.org/message-id/ba261b9fc25dea4069d8ba9a8fcadf35%40postgrespro.ru


--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1

[Robert Haas]

On Tue, Jul 18, 2017 at 9:16 AM, Marina Polyakova
 wrote:
> Here I have made the 5th version of the patches. I have added the
> precalculation of all primitive nodes that don't return set, are not
> volatile themselves and their arguments are constant or precalculated
> expressions too. There're regression tests for all of them and little notes
> in the documentation. Like for the previous patches it seems that there is
> no obvious performance degradation too on regular queries (according to
> pgbench).

pgbench probably isn't a very good test for this sort of thing - it
only issues very short-running queries where the cost of evaluating
expressions is a relatively small part of the total cost.  Even if
things get worse, I'm not sure if you'd see it.  I'm not sure exactly
how you could construct a test case that could be harmed by this patch
- I guess you'd want to initialize lots of CacheExprs but never make
use of the caching usefully?

It could also be useful to test things like TPC-H to see if you get an
improvement.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1

[Marina Polyakova]

Hi Marina,


Hello again!


I still don't see anything particularly wrong with your patch. It
applies, passes all test, it is well test-covered and even documented.
Also I've run `make installcheck` under Valgrind and didn't find any
memory-related errors.


Thank you very much as usual!


Is there anything that you would like to change before we call it more
or less final?


I would like to add some primitive nodes for precalculation if their 
behaviour allows to do it and their arguments/inputs are constant or 
precalculated too; and regression tests for it, of course. Also I would 
like to add some notes about precalculation of stable functions in 
documentation, for example, here [1] and here [2].


Also I would advice to add your branch to our internal buildfarm just 
to

make sure everything is OK on exotic platforms like Windows ;)


Thanks! Done)

[1] https://www.postgresql.org/docs/10/static/xfunc-volatility.html
[2] https://www.postgresql.org/docs/10/static/sql-createfunction.html
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1

[Aleksander Alekseev]

Hi Marina,

I still don't see anything particularly wrong with your patch. It
applies, passes all test, it is well test-covered and even documented.
Also I've run `make installcheck` under Valgrind and didn't find any
memory-related errors.

Is there anything that you would like to change before we call it more
or less final?

Also I would advice to add your branch to our internal buildfarm just to
make sure everything is OK on exotic platforms like Windows ;)

On Mon, May 22, 2017 at 06:32:17PM +0300, Marina Polyakova wrote:
> > Hi,
> 
> Hello!
> 
> > I've not followed this thread, but just scanned this quickly because it
> > affects execExpr* stuff.
> 
> Thank you very much for your comments! Thanks to them I have made v4 of the
> patches (as in the previous one, only planning and execution part is
> changed).
> 
> > Looks like having something like struct CachedExprState would be better,
> > than these separate allocations?  That also allows to aleviate some size
> > concerns when adding new fields (see below)
> 
> > I'd rather not have this on function scope - a) the stack pressure in
> > ExecInterpExpr is quite noticeable in profiles already b) this is going
> > to trigger warnings because of unused vars, because the compiler doesn't
> > understand that EEOP_CACHEDEXPR_IF_CACHED always follows
> > EEOP_CACHEDEXPR_SUBEXPR_END.
> > 
> > How about instead storing oldcontext in the expression itself?
> 
> Thanks, in new version I did all of it in this way.
> 
> > I'm also not sure how acceptable it is to just assume it's ok to leave
> > stuff in per_query_memory, in some cases that could prove to be
> > problematic.
> 
> I agree with you and in new version context is changed only for copying
> datum of result value (if it's a pointer, its data should be allocated in
> per_query_memory, or we will lost it for next tuples).
> 
> > Is this actually a meaningful path?  Shouldn't always have done const
> > evaluation before adding CachedExpr's?
> 
> eval_const_expressions_mutator is used several times, and one of them in
> functions for selectivity evaluation (set_baserel_size_estimates ->
> clauselist_selectivity -> clause_selectivity -> restriction_selectivity ->
> ... -> get_restriction_variable -> estimate_expression_value ->
> eval_const_expressions_mutator). In set_baserel_size_estimates function
> right after selectivity evaluation there's costs evaluation and cached
> expressions should be replaced before costs. I'm not sure that it is a good
> idea to insert cached expressions replacement in set_baserel_size_estimates,
> because in comments to it it's said "The rel's targetlist and restrictinfo
> list must have been constructed already, and rel->tuples must be set." and
> its file costsize.c is entitled as "Routines to compute (and set) relation
> sizes and path costs". So I have inserted cached expressions replacement
> just before it (but I'm not sure that I have seen all places where it should
> be inserted). What do you think about all of this?
> 
> -- 
> Marina Polyakova
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company

> From 02262b9f3a3215d3884b6ac188bafa6517ac543d Mon Sep 17 00:00:00 2001
> From: Marina Polyakova 
> Date: Mon, 15 May 2017 14:24:36 +0300
> Subject: [PATCH v4 1/3] Precalculate stable functions, infrastructure
> 
> Now in Postgresql only immutable functions are precalculated; stable functions
> are calculated for every row so in fact they don't differ from volatile
> functions.
> 
> This patch includes:
> - creation of CachedExpr node
> - usual node functions for it
> - mutator to replace nonovolatile functions' and operators' expressions by
> appropriate cached expressions.
> ---
>  src/backend/nodes/copyfuncs.c|  31 +
>  src/backend/nodes/equalfuncs.c   |  31 +
>  src/backend/nodes/nodeFuncs.c| 151 
>  src/backend/nodes/outfuncs.c |  56 
>  src/backend/nodes/readfuncs.c|  48 +++
>  src/backend/optimizer/plan/planner.c | 259 
> +++
>  src/include/nodes/nodeFuncs.h|   1 +
>  src/include/nodes/nodes.h|   1 +
>  src/include/nodes/primnodes.h|  38 +
>  9 files changed, 616 insertions(+)
> 
> diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
> index 6ad3844..f9f69a1 100644
> --- a/src/backend/nodes/copyfuncs.c
> +++ b/src/backend/nodes/copyfuncs.c
> @@ -1527,6 +1527,34 @@ _copyNullIfExpr(const NullIfExpr *from)
>   return newnode;
>  }
>  
> +static CachedExpr *
> +_copyCachedExpr(const CachedExpr *from)
> +{
> + CachedExpr *newnode = makeNode(CachedExpr);
> +
> + COPY_SCALAR_FIELD(subexprtype);
> + switch(from->subexprtype)
> + {
> + case CACHED_FUNCEXPR:
> + COPY_NODE_FIELD(subexpr.funcexpr);
> + break;
> + case CACHED_OPEXPR:
> + 

Re: [HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1

[Andres Freund]

Hi,


On 2017-05-18 19:00:09 +0300, Marina Polyakova wrote:
> > Here's v2 of the patches. Changes from v1:
> 
> And here there's v3 of planning and execution: common executor steps for all
> types of cached expression.

I've not followed this thread, but just scanned this quickly because it
affects execExpr* stuff.

> + case T_CachedExpr:
> + {
> + int adjust_jump;
> +
> + /*
> +  * Allocate and fill scratch memory used by all 
> steps of
> +  * CachedExpr evaluation.
> +  */
> + scratch.d.cachedexpr.isExecuted = (bool *) 
> palloc(sizeof(bool));
> + scratch.d.cachedexpr.resnull = (bool *) 
> palloc(sizeof(bool));
> + scratch.d.cachedexpr.resvalue = (Datum *) 
> palloc(sizeof(Datum));
> +
> + *scratch.d.cachedexpr.isExecuted = false;
> + *scratch.d.cachedexpr.resnull = false;
> + *scratch.d.cachedexpr.resvalue = (Datum) 0;

Looks like having something like struct CachedExprState would be better,
than these separate allocations?  That also allows to aleviate some size
concerns when adding new fields (see below)


> @@ -279,6 +279,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, 
> bool *isnull)
>   TupleTableSlot *innerslot;
>   TupleTableSlot *outerslot;
>   TupleTableSlot *scanslot;
> + MemoryContext oldContext;   /* for EEOP_CACHEDEXPR_* */

I'd rather not have this on function scope - a) the stack pressure in
ExecInterpExpr is quite noticeable in profiles already b) this is going
to trigger warnings because of unused vars, because the compiler doesn't
understand that EEOP_CACHEDEXPR_IF_CACHED always follows
EEOP_CACHEDEXPR_SUBEXPR_END.

How about instead storing oldcontext in the expression itself?

I'm also not sure how acceptable it is to just assume it's ok to leave
stuff in per_query_memory, in some cases that could prove to be
problematic.


> + case T_CachedExpr:
> + {
> + CachedExpr *cachedexpr = (CachedExpr *) node;
> + Node   *new_subexpr = 
> eval_const_expressions_mutator(
> + get_subexpr(cachedexpr), context);
> + CachedExpr *new_cachedexpr;
> +
> + /*
> +  * If unsafe transformations are used cached 
> expression should
> +  * be always simplified.
> +  */
> + if (context->estimate)
> + Assert(IsA(new_subexpr, Const));
> +
> + if (IsA(new_subexpr, Const))
> + {
> + /* successfully simplified it */
> + return new_subexpr; 
> + }
> + else
> + {
> + /*
> +  * The expression cannot be simplified 
> any further, so build
> +  * and return a replacement CachedExpr 
> node using the
> +  * possibly-simplified arguments of 
> subexpression.
> +  */

Is this actually a meaningful path?  Shouldn't always have done const
evaluation before adding CachedExpr's?


Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1

[Marina Polyakova]

Here's v2 of the patches. Changes from v1:


And here there's v3 of planning and execution: common executor steps for 
all types of cached expression.


--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres CompanyFrom 5e89221251670526eb2b5750868ac73eee48f10b Mon Sep 17 00:00:00 2001
From: Marina Polyakova 
Date: Mon, 15 May 2017 15:31:21 +0300
Subject: [PATCH 2/3] Precalculate stable functions, planning and execution v3

Now in Postgresql only immutable functions are precalculated; stable functions
are calculated for every row so in fact they don't differ from volatile
functions.

This patch includes:
- replacement nonvolatile functions and operators by appropriate cached
expressions
- planning and execution cached expressions
- regression tests
---
 src/backend/executor/execExpr.c|   37 +
 src/backend/executor/execExprInterp.c  |   37 +
 src/backend/optimizer/path/allpaths.c  |9 +-
 src/backend/optimizer/path/clausesel.c |   13 +
 src/backend/optimizer/plan/planagg.c   |1 +
 src/backend/optimizer/plan/planner.c   |   28 +
 src/backend/optimizer/util/clauses.c   |   55 +
 src/backend/utils/adt/ruleutils.c  |5 +
 src/include/executor/execExpr.h|   19 +
 src/include/optimizer/planner.h|3 +
 src/include/optimizer/tlist.h  |8 +-
 src/pl/plpgsql/src/pl_exec.c   |   10 +
 .../expected/precalculate_stable_functions.out | 2625 
 src/test/regress/serial_schedule   |1 +
 .../regress/sql/precalculate_stable_functions.sql  |  949 +++
 15 files changed, 3797 insertions(+), 3 deletions(-)
 create mode 100644 src/test/regress/expected/precalculate_stable_functions.out
 create mode 100644 src/test/regress/sql/precalculate_stable_functions.sql

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 5a34a46..3c2068d 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -865,6 +865,43 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state,
 break;
 			}
 
+		case T_CachedExpr:
+			{
+int 		adjust_jump;
+
+/*
+ * Allocate and fill scratch memory used by all steps of
+ * CachedExpr evaluation.
+ */
+scratch.d.cachedexpr.isExecuted = (bool *) palloc(sizeof(bool));
+scratch.d.cachedexpr.resnull = (bool *) palloc(sizeof(bool));
+scratch.d.cachedexpr.resvalue = (Datum *) palloc(sizeof(Datum));
+
+*scratch.d.cachedexpr.isExecuted = false;
+*scratch.d.cachedexpr.resnull = false;
+*scratch.d.cachedexpr.resvalue = (Datum) 0;
+scratch.d.cachedexpr.jumpdone = -1;
+
+/* add EEOP_CACHEDEXPR_IF_CACHED step */
+scratch.opcode = EEOP_CACHEDEXPR_IF_CACHED;
+ExprEvalPushStep(state, );
+adjust_jump = state->steps_len - 1;
+
+/* add subexpression steps */
+ExecInitExprRec((Expr *) get_subexpr((CachedExpr *) node),
+parent, state, resv, resnull);
+
+/* add EEOP_CACHEDEXPR_SUBEXPR_END step */
+scratch.opcode = EEOP_CACHEDEXPR_SUBEXPR_END;
+ExprEvalPushStep(state, );
+
+/* adjust jump target */
+state->steps[adjust_jump].d.cachedexpr.jumpdone =
+	state->steps_len;
+
+break;
+			}
+
 		case T_ScalarArrayOpExpr:
 			{
 ScalarArrayOpExpr *opexpr = (ScalarArrayOpExpr *) node;
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index fed0052..ac7b7f8 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -279,6 +279,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 	TupleTableSlot *innerslot;
 	TupleTableSlot *outerslot;
 	TupleTableSlot *scanslot;
+	MemoryContext oldContext;	/* for EEOP_CACHEDEXPR_* */
 
 	/*
 	 * This array has to be in the same order as enum ExprEvalOp.
@@ -309,6 +310,8 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 		&_EEOP_FUNCEXPR_STRICT,
 		&_EEOP_FUNCEXPR_FUSAGE,
 		&_EEOP_FUNCEXPR_STRICT_FUSAGE,
+		&_EEOP_CACHEDEXPR_IF_CACHED,
+		&_EEOP_CACHEDEXPR_SUBEXPR_END,
 		&_EEOP_BOOL_AND_STEP_FIRST,
 		&_EEOP_BOOL_AND_STEP,
 		&_EEOP_BOOL_AND_STEP_LAST,
@@ -721,6 +724,40 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 			EEO_NEXT();
 		}
 
+		EEO_CASE(EEOP_CACHEDEXPR_IF_CACHED)
+		{
+			if (*op->d.cachedexpr.isExecuted)
+			{
+/* use saved result and skip subexpression evaluation */
+*op->resnull = *op->d.cachedexpr.resnull;
+if (!(*op->resnull))
+	*op->resvalue = *op->d.cachedexpr.resvalue;
+
+EEO_JUMP(op->d.cachedexpr.jumpdone);
+			}
+
+			/*
+			 * Switch per-query memory context for subexpression evaluation.
+			 * It is necessary to save result between all tuples.
+			 */
+			oldContext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
+
+			

Re: [HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1

[Marina Polyakova]

Hello, Aleksander!


I've noticed that this patch needs a review and decided to take a look.


Thank you very much!


There are a trailing whitespaces,
  though.


Oh, sorry, I'll check them.


I see 8-10% performance improvement on full text search queries.


Glad to hear it =)


It seems that there is no obvious performance degradation on regular
  queries (according to pgbench).


Thanks for testing it, I'll try not to forget about it next time =[


In short, it looks very promising.


And thanks again!

--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1

[Aleksander Alekseev]

Hi Marina,

I've noticed that this patch needs a review and decided to take a look.
Here is a short summary:

* Patches apply to the master branch. There are a trailing whitespaces,
  though.
* All tests pass.
* I see 8-10% performance improvement on full text search queries.
* It seems that there is no obvious performance degradation on regular
  queries (according to pgbench).

In short, it looks very promising. 

-- 
Best regards,
Aleksander Alekseev


signature.asc
Description: PGP signature

Re: [HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1

[Alexander Korotkov]

On Thu, May 4, 2017 at 7:51 PM, Marina Polyakova  wrote:

> and here I send infrastructure patch which includes <...>
>>
>
> Next 2 patches:
>
> Patch 'planning and execution', which includes:
> - replacement nonvolatile functions and operators by appropriate cached
> expressions;
> - planning and execution cached expressions;
> - regression tests.
>
> Patch 'costs', which includes cost changes for cached expressions
> (according to their behaviour).


Great, thank you for your work.
It's good and widely used practice to prepend number to the patch name
while dealing with patch set.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1

[Marina Polyakova]

and here I send infrastructure patch which includes <...>


Next 2 patches:

Patch 'planning and execution', which includes:
- replacement nonvolatile functions and operators by appropriate cached 
expressions;

- planning and execution cached expressions;
- regression tests.

Patch 'costs', which includes cost changes for cached expressions 
(according to their behaviour).


--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres CompanyFrom cf446cbfc8625701f9e3f32d1870b47de869802a Mon Sep 17 00:00:00 2001
From: Marina Polyakova 
Date: Thu, 4 May 2017 19:36:05 +0300
Subject: [PATCH 3/3] Precalculate stable functions, costs v1

Now in Postgresql only immutable functions are precalculated; stable functions
are calculated for every row so in fact they don't differ from volatile
functions.

This patch includes:
- cost changes for cached expressions (according to their behaviour)
---
 src/backend/optimizer/path/costsize.c | 58 ---
 1 file changed, 53 insertions(+), 5 deletions(-)

diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 52643d0..34707fa 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -140,6 +140,7 @@ static MergeScanSelCache *cached_scansel(PlannerInfo *root,
 			   PathKey *pathkey);
 static void cost_rescan(PlannerInfo *root, Path *path,
 			Cost *rescan_startup_cost, Cost *rescan_total_cost);
+static double cost_eval_cacheable_expr_per_tuple(Node *node);
 static bool cost_qual_eval_walker(Node *node, cost_qual_eval_context *context);
 static void get_restriction_qual_cost(PlannerInfo *root, RelOptInfo *baserel,
 		  ParamPathInfo *param_info,
@@ -3464,6 +3465,44 @@ cost_qual_eval_node(QualCost *cost, Node *qual, PlannerInfo *root)
 	*cost = context.total;
 }
 
+/*
+ * cost_eval_cacheable_expr_per_tuple
+ *		Evaluate per tuple cost for expressions that can be cacheable.
+ *
+ * This function was created to not duplicate code for some expression and
+ * cached some expression.
+ */
+static double
+cost_eval_cacheable_expr_per_tuple(Node *node)
+{
+	double result;
+
+	/*
+	 * For each operator or function node in the given tree, we charge the
+	 * estimated execution cost given by pg_proc.procost (remember to multiply
+	 * this by cpu_operator_cost).
+	 */
+	if (IsA(node, FuncExpr))
+	{
+		result = get_func_cost(((FuncExpr *) node)->funcid) * cpu_operator_cost;
+	}
+	else if (IsA(node, OpExpr))
+	{
+		OpExpr *opexpr = (OpExpr *) node;
+
+		/* rely on struct equivalence to treat these all alike */
+		set_opfuncid(opexpr);
+
+		result = get_func_cost(opexpr->opfuncid) * cpu_operator_cost;
+	}
+	else
+	{
+		elog(ERROR, "non cacheable expression node type: %d", (int) nodeTag(node));
+	}
+
+	return result;
+}
+
 static bool
 cost_qual_eval_walker(Node *node, cost_qual_eval_context *context)
 {
@@ -3537,13 +3576,22 @@ cost_qual_eval_walker(Node *node, cost_qual_eval_context *context)
 	 * moreover, since our rowcount estimates for functions tend to be pretty
 	 * phony, the results would also be pretty phony.
 	 */
-	if (IsA(node, FuncExpr))
+	if (IsA(node, FuncExpr) ||
+		IsA(node, OpExpr))
 	{
-		context->total.per_tuple +=
-			get_func_cost(((FuncExpr *) node)->funcid) * cpu_operator_cost;
+		context->total.per_tuple += cost_eval_cacheable_expr_per_tuple(node);
+	}
+	else if (IsA(node, CachedExpr))
+	{	
+		/* 
+		 * Calculate subexpression cost per tuple as usual and add it to startup
+		 * cost (because subexpression will be executed only once for all
+		 * tuples).
+		 */
+		context->total.startup += cost_eval_cacheable_expr_per_tuple(
+			get_subexpr((CachedExpr *) node));
 	}
-	else if (IsA(node, OpExpr) ||
-			 IsA(node, DistinctExpr) ||
+	else if (IsA(node, DistinctExpr) ||
 			 IsA(node, NullIfExpr))
 	{
 		/* rely on struct equivalence to treat these all alike */
-- 
1.9.1

From 508f8b959ff9d1ab78dfc79ab4657b4c10a11690 Mon Sep 17 00:00:00 2001
From: Marina Polyakova 
Date: Thu, 4 May 2017 19:09:51 +0300
Subject: [PATCH 2/3] Precalculate stable functions, planning and execution v1

Now in Postgresql only immutable functions are precalculated; stable functions
are calculated for every row so in fact they don't differ from volatile
functions.

This patch includes:
- replacement nonvolatile functions and operators by appropriate cached
expressions
- planning and execution cached expressions
- regression tests
---
 src/backend/executor/execExpr.c|  70 ++
 src/backend/executor/execExprInterp.c  | 191 +
 src/backend/optimizer/path/allpaths.c  |   9 +-
 src/backend/optimizer/path/clausesel.c |  13 +
 src/backend/optimizer/plan/planagg.c   |   1 +
 src/backend/optimizer/plan/planner.c   |  28 +
 src/backend/optimizer/util/clauses.c   |  43 ++
 src/backend/utils/adt/ruleutils.c