> On 3 June 2018 at 19:11, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Dmitry Dolgov <9erthali...@gmail.com> writes:
>> Just to clarify for myself, for evaluating any stable function here would it 
>> be
>> enough to handle all function-like expressions (FuncExpr / OpExpr /
>> DistinctExpr / NullIfExpr) and check a corresponding function for 
>> provolatile,
>> like in the attached patch?
>
> I think the entire approach is wrong here.  Rather than concerning
> yourself with Params, or any other specific expression type, you
> should be using !contain_volatile_functions() to decide whether
> an expression is run-time-constant.  If it is, use the regular
> expression evaluation machinery to extract the value.

Yes, it makes sense. Then, to my understanding, the attached code is close to
what was described above (although I'm not sure about the Const part).
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 58ec2a6..f02bd30 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -2705,6 +2705,9 @@ pull_partkey_params(PartitionPruneInfo *pinfo, List *steps)
 		{
 			Expr	   *expr = lfirst(lc2);
 
+			if (contain_volatile_functions((Node *) expr))
+				continue;
+
 			if (IsA(expr, Param))
 			{
 				Param	   *param = (Param *) expr;
@@ -2727,6 +2730,13 @@ pull_partkey_params(PartitionPruneInfo *pinfo, List *steps)
 				}
 				gotone = true;
 			}
+			else if (!IsA(expr, Const))
+			{
+				Param	*param = (Param *) expr;
+				pinfo->execparams = bms_add_member(pinfo->execparams,
+												   param->paramid);
+				gotone = true;
+			}
 		}
 	}
 
@@ -3031,37 +3041,35 @@ static bool
 partkey_datum_from_expr(PartitionPruneContext *context,
 						Expr *expr, int stateidx, Datum *value)
 {
-	switch (nodeTag(expr))
-	{
-		case T_Const:
-			*value = ((Const *) expr)->constvalue;
-			return true;
-
-		case T_Param:
-
-			/*
-			 * When being called from the executor we may be able to evaluate
-			 * the Param's value.
-			 */
-			if (context->planstate &&
-				bms_is_member(((Param *) expr)->paramid, context->safeparams))
-			{
-				ExprState  *exprstate;
-				ExprContext *ectx;
-				bool		isNull;
+	if (contain_volatile_functions((Node *) expr))
+		return false;
 
-				exprstate = context->exprstates[stateidx];
-				ectx = context->planstate->ps_ExprContext;
-				*value = ExecEvalExprSwitchContext(exprstate, ectx, &isNull);
-				if (isNull)
-					return false;
+	if (IsA(expr, Const))
+	{
+		*value = ((Const *) expr)->constvalue;
+		return true;
+	}
+	else
+	{
+		/*
+		 * When being called from the executor we may be able to evaluate
+		 * the Param's value.
+		 */
+		if (context->planstate &&
+			bms_is_member(((Param *) expr)->paramid, context->safeparams))
+		{
+			ExprState  *exprstate;
+			ExprContext *ectx;
+			bool		isNull;
 
-				return true;
-			}
-			break;
+			exprstate = context->exprstates[stateidx];
+			ectx = context->planstate->ps_ExprContext;
+			*value = ExecEvalExprSwitchContext(exprstate, ectx, &isNull);
+			if (isNull)
+				return false;
 
-		default:
-			break;
+			return true;
+		}
 	}
 
 	return false;

Reply via email to