Hi,
This is a proof of concept patch for recognizing stable function calls
with constant arguments and only calling them once per execution. I'm
posting it to the list to gather feedback whether this is a dead end
or not.
Last time when this was brought up on the list, Tom Lane commented
that doing the checks for constant folding probably outweigh the
gains. http://archives.postgresql.org/pgsql-hackers/2010-11/msg01641.php
Yesterday I was looking at evaluate_function(), where currently
function constant folding happens, and realized that most of the
knowledge needed to implement this for FuncExpr and OpExpr (and
combinations thereof) is already there. This would probably cover 90%
of the use cases with very little overhead. Other expression types
can't be supported very well with this approach, notably case
expressions and AND/OR/NOT.
However, the code isn't very pretty. It works by changing
evaluate_function() to recognize stable/immutable functions with
constant stable/immutable arguments that aren't eligible for constant
folding. It sets a new boolean attribute FuncExpr.stableconst which is
checked during the function's first execution. If true, the result is
stored in ExprState and evalfunc is replaced with a new one that
returns the cached result.
Most common use cases cover timestamptz expressions (because they are
never immutable) such as:
ts>=to_timestamp('2010-01-01', 'YYYY-MM-DD')
ts>=(now() - interval '10 days')
Typically an index on the column would marginalize performance lost by
repeatedly evaluating the function.
----
Test data (1,051,777 rows, 38MB):
create table ts as select generate_series(timestamptz '2001-01-01',
'2011-01-01', '5min') ts;
Test query (using pgbench -T 30, taking the best of 3 runs):
select * from ts where ts>=to_timestamp('2001-01-01', 'YYYY-MM-DD')
and ts<=to_timestamp('2001-01-01', 'YYYY-MM-DD');
Unpatched tps = 0.927458
Patched tps = 6.729378
There's even a measurable improvement from caching now() calls:
select * from ts where ts>now();
Unpatched tps = 8.106439
Patched tps = 9.401684
Regards,
Marti
diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index 80f08d8..562e43d 100644
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -40,6 +40,7 @@
#include "access/tupconvert.h"
#include "catalog/pg_type.h"
#include "commands/typecmds.h"
+#include "executor/executor.h"
#include "executor/execdebug.h"
#include "executor/nodeSubplan.h"
#include "funcapi.h"
@@ -95,6 +96,14 @@ static void ExecPrepareTuplestoreResult(FuncExprState *fcache,
Tuplestorestate *resultStore,
TupleDesc resultDesc);
static void tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc);
+static void cache_function_result(FuncExprState *fcache,
+ ExprContext *econtext,
+ bool *isNull,
+ ExprDoneCond *isDone);
+static Datum ExecCachedFunctionResult(FuncExprState *fcache,
+ ExprContext *econtext,
+ bool *isNull,
+ ExprDoneCond *isDone);
static Datum ExecMakeFunctionResult(FuncExprState *fcache,
ExprContext *econtext,
bool *isNull,
@@ -1522,6 +1531,44 @@ tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc)
}
/*
+ * cache_function_result
+ */
+static void
+cache_function_result(FuncExprState *fcache,
+ ExprContext *econtext,
+ bool *isNull,
+ ExprDoneCond *isDone)
+{
+ MemoryContext oldcontext;
+
+ /* This cache has to persist for the whole query */
+ oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
+
+ fcache->cachedResult = ExecMakeFunctionResult(fcache, econtext, isNull, isDone);
+ fcache->cachedIsNull = *isNull;
+
+ /* Set-returning functions can't be cached */
+ Assert(!isDone || *isDone == ExprSingleResult);
+
+ MemoryContextSwitchTo(oldcontext);
+}
+
+/*
+ * ExecCachedFunctionResult
+ */
+static Datum
+ExecCachedFunctionResult(FuncExprState *fcache,
+ ExprContext *econtext,
+ bool *isNull,
+ ExprDoneCond *isDone)
+{
+ if(isDone)
+ *isDone = ExprSingleResult;
+ *isNull = fcache->cachedIsNull;
+ return fcache->cachedResult;
+}
+
+/*
* ExecMakeFunctionResult
*
* Evaluate the arguments to a function and then the function itself.
@@ -2257,10 +2304,18 @@ ExecEvalFunc(FuncExprState *fcache,
init_fcache(func->funcid, func->inputcollid, fcache,
econtext->ecxt_per_query_memory, true);
- /* Go directly to ExecMakeFunctionResult on subsequent uses */
- fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResult;
+ if(func->stableconst)
+ {
+ cache_function_result(fcache, econtext, isNull, isDone);
+
+ /* XXX Since we only need function cache once, should we clean it up now? */
+ fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecCachedFunctionResult;
+ }
+ else
+ /* Go directly to ExecMakeFunctionResult on subsequent uses */
+ fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResult;
- return ExecMakeFunctionResult(fcache, econtext, isNull, isDone);
+ return ExecEvalExpr((ExprState *) fcache, econtext, isNull, isDone);
}
/* ----------------------------------------------------------------
@@ -2280,10 +2335,18 @@ ExecEvalOper(FuncExprState *fcache,
init_fcache(op->opfuncid, op->inputcollid, fcache,
econtext->ecxt_per_query_memory, true);
- /* Go directly to ExecMakeFunctionResult on subsequent uses */
- fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResult;
+ if(op->stableconst)
+ {
+ cache_function_result(fcache, econtext, isNull, isDone);
+
+ /* XXX Since we only need function cache once, should we clean it up now? */
+ fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecCachedFunctionResult;
+ }
+ else
+ /* Go directly to ExecMakeFunctionResult on subsequent uses */
+ fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResult;
- return ExecMakeFunctionResult(fcache, econtext, isNull, isDone);
+ return ExecEvalExpr((ExprState *) fcache, econtext, isNull, isDone);
}
/* ----------------------------------------------------------------
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 661a516..cb6a4e8 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -1183,6 +1183,7 @@ _copyFuncExpr(FuncExpr *from)
COPY_SCALAR_FIELD(inputcollid);
COPY_NODE_FIELD(args);
COPY_LOCATION_FIELD(location);
+ COPY_LOCATION_FIELD(stableconst);
return newnode;
}
@@ -1219,6 +1220,7 @@ _copyOpExpr(OpExpr *from)
COPY_SCALAR_FIELD(inputcollid);
COPY_NODE_FIELD(args);
COPY_LOCATION_FIELD(location);
+ COPY_LOCATION_FIELD(stableconst);
return newnode;
}
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index 4d2eccf..4c4c909 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -465,6 +465,7 @@ makeFuncExpr(Oid funcid, Oid rettype, List *args,
funcexpr->inputcollid = inputcollid;
funcexpr->args = args;
funcexpr->location = -1;
+ funcexpr->stableconst = false;
return funcexpr;
}
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index baa90fa..521bad2 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -120,9 +120,9 @@ static List *add_function_defaults(List *args, Oid result_type,
static List *fetch_function_defaults(HeapTuple func_tuple);
static void recheck_cast_function_args(List *args, Oid result_type,
HeapTuple func_tuple);
-static Expr *evaluate_function(Oid funcid, Oid result_type, int32 result_typmod,
- Oid result_collid, Oid input_collid, List *args,
- HeapTuple func_tuple,
+static Expr *evaluate_function(Expr *oldexpr, Oid funcid, Oid result_type,
+ int32 result_typmod, Oid result_collid, Oid input_collid,
+ List *args, HeapTuple func_tuple,
eval_const_expressions_context *context);
static Expr *inline_function(Oid funcid, Oid result_type, Oid result_collid,
Oid input_collid, List *args,
@@ -3413,7 +3413,7 @@ simplify_function(Expr *oldexpr, Oid funcid,
Oid transform;
/*
- * We have three strategies for simplification: execute the function to
+ * XXX We have three strategies for simplification: execute the function to
* deliver a constant result, use a transform function to generate a
* substitute node tree, or expand in-line the body of the function
* definition (which only works for simple SQL-language functions, but
@@ -3434,7 +3434,7 @@ simplify_function(Expr *oldexpr, Oid funcid,
else if (((Form_pg_proc) GETSTRUCT(func_tuple))->pronargs > list_length(*args))
*args = add_function_defaults(*args, result_type, func_tuple, context);
- newexpr = evaluate_function(funcid, result_type, result_typmod,
+ newexpr = evaluate_function(oldexpr, funcid, result_type, result_typmod,
result_collid, input_collid, *args,
func_tuple, context);
@@ -3712,7 +3712,7 @@ recheck_cast_function_args(List *args, Oid result_type, HeapTuple func_tuple)
/*
* evaluate_function: try to pre-evaluate a function call
*
- * We can do this if the function is strict and has any constant-null inputs
+ * XXX We can do this if the function is strict and has any constant-null inputs
* (just return a null constant), or if the function is immutable and has all
* constant inputs (call it and return the result as a Const node). In
* estimation mode we are willing to pre-evaluate stable functions too.
@@ -3720,15 +3720,17 @@ recheck_cast_function_args(List *args, Oid result_type, HeapTuple func_tuple)
* Returns a simplified expression if successful, or NULL if cannot
* simplify the function.
*/
+/* XXX rename this function */
static Expr *
-evaluate_function(Oid funcid, Oid result_type, int32 result_typmod,
- Oid result_collid, Oid input_collid, List *args,
- HeapTuple func_tuple,
+evaluate_function(Expr *oldexpr, Oid funcid, Oid result_type,
+ int32 result_typmod, Oid result_collid, Oid input_collid,
+ List *args, HeapTuple func_tuple,
eval_const_expressions_context *context)
{
Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple);
bool has_nonconst_input = false;
bool has_null_input = false;
+ bool has_non_stableconst_input = false;
ListCell *arg;
FuncExpr *newexpr;
@@ -3757,10 +3759,21 @@ evaluate_function(Oid funcid, Oid result_type, int32 result_typmod,
*/
foreach(arg, args)
{
- if (IsA(lfirst(arg), Const))
- has_null_input |= ((Const *) lfirst(arg))->constisnull;
+ Expr *expr = (Expr *) lfirst(arg);
+
+ if (IsA(expr, Const))
+ has_null_input |= ((Const *) expr)->constisnull;
else
+ {
has_nonconst_input = true;
+
+ if (IsA(expr, FuncExpr))
+ has_non_stableconst_input |= !((FuncExpr *) expr)->stableconst;
+ else if (IsA(expr, OpExpr))
+ has_non_stableconst_input |= !((OpExpr *) expr)->stableconst;
+ else
+ has_non_stableconst_input = true;
+ }
}
/*
@@ -3778,37 +3791,69 @@ evaluate_function(Oid funcid, Oid result_type, int32 result_typmod,
* non-strict function, constant NULL inputs are treated the same as
* constant non-NULL inputs.)
*/
- if (has_nonconst_input)
+ if (has_non_stableconst_input)
return NULL;
/*
- * Ordinarily we are only allowed to simplify immutable functions. But for
+ * XXX Ordinarily we are only allowed to simplify immutable functions. But for
* purposes of estimation, we consider it okay to simplify functions that
* are merely stable; the risk that the result might change from planning
* time to execution time is worth taking in preference to not being able
* to estimate the value at all.
*/
- if (funcform->provolatile == PROVOLATILE_IMMUTABLE)
- /* okay */ ;
- else if (context->estimate && funcform->provolatile == PROVOLATILE_STABLE)
- /* okay */ ;
- else
+ if (funcform->provolatile == PROVOLATILE_VOLATILE)
return NULL;
+ /* Arguments are not plan-time constants, but stable execution-time
+ * constants -- or the function itself is stable
+ */
+ if((!context->estimate) && (has_nonconst_input ||
+ funcform->provolatile == PROVOLATILE_STABLE))
+ {
+ Expr *expr;
+
+ /*
+ * We want to keep the original expression type, because EXPLAIN will
+ * show these expressions
+ */
+ if(oldexpr)
+ expr = copyObject(oldexpr);
+ else
+ /*
+ * XXX This can make EXPLAIN ANALYZE VERBOSE ugly!
+ * explain analyze verbose select current_date;
+ */
+ expr = makeFuncExpr(funcid, result_type, args, result_collid,
+ input_collid, COERCE_DONTCARE);
+
+ /*
+ * XXX If we don't mutate 'args' here, we prevent redundant caching
+ * of argument expressions. Does this have any downsides?
+ */
+ if (IsA(expr, FuncExpr))
+ {
+ ((FuncExpr *) expr)->stableconst = true;
+ ((FuncExpr *) expr)->args = args;
+ }
+ else if(IsA(expr, OpExpr))
+ {
+ ((OpExpr *) expr)->stableconst = true;
+ ((OpExpr *) expr)->args = args;
+ }
+ else
+ Assert(false); /* XXX */
+
+ return (Expr *) expr;
+ }
+
/*
* OK, looks like we can simplify this operator/function.
*
* Build a new FuncExpr node containing the already-simplified arguments.
*/
- newexpr = makeNode(FuncExpr);
- newexpr->funcid = funcid;
- newexpr->funcresulttype = result_type;
- newexpr->funcretset = false;
- newexpr->funcformat = COERCE_DONTCARE; /* doesn't matter */
- newexpr->funccollid = result_collid; /* doesn't matter */
- newexpr->inputcollid = input_collid;
- newexpr->args = args;
- newexpr->location = -1;
+
+ newexpr = makeFuncExpr(funcid, result_type, args, result_collid,
+ input_collid, COERCE_DONTCARE);
return evaluate_expr((Expr *) newexpr, result_type, result_typmod,
result_collid);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index b3eed7d..8238464 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -624,6 +624,13 @@ typedef struct FuncExprState
FmgrInfo func;
/*
+ * These are used for holding the cached result of stable function calls
+ * with constant arguments.
+ */
+ Datum cachedResult;
+ bool cachedIsNull;
+
+ /*
* For a set-returning function (SRF) that returns a tuplestore, we keep
* the tuplestore here and dole out the result rows one at a time. The
* slot holds the row currently being returned.
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index f1e20ef..af0c6a7 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -335,6 +335,7 @@ typedef struct FuncExpr
Oid inputcollid; /* OID of collation that function should use */
List *args; /* arguments to the function */
int location; /* token location, or -1 if unknown */
+ bool stableconst; /* cache the result for each ExprState? */
} FuncExpr;
/*
@@ -380,6 +381,7 @@ typedef struct OpExpr
Oid inputcollid; /* OID of collation that operator should use */
List *args; /* arguments to the operator (1 or 2) */
int location; /* token location, or -1 if unknown */
+ bool stableconst; /* cache the result for each ExprState? */
} OpExpr;
/*
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
