Improve gapfill row count estimate (#9735)

Fixes #9716: bad Gapfill row count estimate of 0 (due to 0 original rows
which ignores all the gap fills generating a lot of rows) leading to bad
plans and bad performance.
It's not a bug fix but an enhancement.

We can estimate Gapfill row count much better if we can evaluate
start/finish/period at planning time and use more accurate row count
estimate of `(subplan rows) + (finish-start)/period` instead of just
`subplan rows`.

We may have maximum of original rows plus filled gaps in addition to
those rows, and this is the estimate used here.

Author: natalya-aksman

.unreleased/pr_9735

@@ -0,0 +1,2 @@
+Implements: #9735 Improve GapFill row count estimate 
+Thanks: @Fabian-2596 for suggesting more accurate GapFill row count estimate

tsl/src/nodes/gapfill/gapfill.h

@@ -8,6 +8,7 @@
 #include <postgres.h>
 #include <nodes/pathnodes.h>
 #include <nodes/primnodes.h>
+#include <utils/builtins.h>
 
 #define GAPFILL_FUNCTION "time_bucket_gapfill"
 #define GAPFILL_LOCF_FUNCTION "locf"
@@ -34,3 +35,29 @@ typedef struct GapFillPath
 	CustomPath cpath;
 	FuncExpr *func; /* time_bucket_gapfill function call */
 } GapFillPath;
+
+typedef enum GapFillBoundary
+{
+	GAPFILL_START,
+	GAPFILL_END,
+} GapFillBoundary;
+
+typedef struct GapFillArgEvalContext
+{
+	PlannerInfo *root;		/* needed for evaluation at planning time */
+	CustomScanState *state; /* needed for evaluation at execution time */
+	Oid typid;				/* gapfill function type */
+	List *args;				/* gapfill function arguments */
+	FromExpr *jt;			/* needed for inferring boundaries from WHERE */
+	bool estimate_failed;	/* whether estimation during planning failed */
+} GapFillArgEvalContext;
+
+extern int64 infer_gapfill_boundary(GapFillArgEvalContext *gapfill_ctx, GapFillBoundary boundary);
+extern Datum gapfill_estimate_arg(GapFillArgEvalContext *gapfill_plan_ctx, Node *arg, bool *isnull);
+
+extern bool gapfill_is_const_null(Expr *expr);
+extern bool gapfill_is_simple_expr(Expr *node);
+extern Const *make_const_value_for_gapfill_internal(Oid typid, int64 value);
+extern int64 gapfill_datum_get_internal(Datum, Oid);
+extern int64 gapfill_bucket_width_get_internal(Oid timetype, Oid argtype, Datum arg,
+											   Interval **interval);

tsl/src/nodes/gapfill/gapfill_exec.c

@@ -28,19 +28,14 @@
 #include <utils/typcache.h>
 
 #include <compat/compat.h>
+#include "estimate.h"
 #include "gapfill.h"
 #include "gapfill_internal.h"
 #include "interpolate.h"
 #include "locf.h"
 #include "time_bucket.h"
 #include <annotations.h>
 
-typedef enum GapFillBoundary
-{
-	GAPFILL_START,
-	GAPFILL_END,
-} GapFillBoundary;
-
 typedef union GapFillColumnStateUnion
 {
 	GapFillColumnState *base;
@@ -175,8 +170,8 @@ get_timezone_arg(GapFillState *state)
 	return lthird(state->args);
 }
 
-static inline int64
-gapfill_period_get_internal(Oid timetype, Oid argtype, Datum arg, Interval **interval)
+int64
+gapfill_bucket_width_get_internal(Oid timetype, Oid argtype, Datum arg, Interval **interval)
 {
 	switch (timetype)
 	{
@@ -240,8 +235,8 @@ gapfill_state_create(CustomScan *cscan)
 	return (Node *) state;
 }
 
-static bool
-is_const_null(Expr *expr)
+bool
+gapfill_is_const_null(Expr *expr)
 {
 	return IsA(expr, Const) && castNode(Const, expr)->constisnull;
 }
@@ -288,7 +283,7 @@ var_equal(Var *v1, Var *v2)
 }
 
 static bool
-is_simple_expr_walker(Node *node, void *context)
+gapfill_is_simple_expr_walker(Node *node, void *context)
 {
 	if (node == NULL)
 	{
@@ -328,22 +323,22 @@ is_simple_expr_walker(Node *node, void *context)
 		default:
 			return true;
 	}
-	return expression_tree_walker(node, is_simple_expr_walker, context);
+	return expression_tree_walker(node, gapfill_is_simple_expr_walker, context);
 }
 
 /*
  * check if expression is simple expression and contains only simple
  * subexpressions
  */
-static bool
-is_simple_expr(Expr *node)
+bool
+gapfill_is_simple_expr(Expr *node)
 {
 	/*
 	 * since expression_tree_walker does early exit on true and we use that to
 	 * skip processing on first non-simple expression we invert return value
 	 * from expression_tree_walker here
 	 */
-	return !is_simple_expr_walker((Node *) node, NULL);
+	return !gapfill_is_simple_expr_walker((Node *) node, NULL);
 }
 
 /*
@@ -360,7 +355,7 @@ align_with_time_bucket(GapFillState *state, Expr *expr)
 	Datum value;
 	bool isnull;
 
-	if (!is_simple_expr(expr))
+	if (!gapfill_is_simple_expr(expr))
 	{
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -399,39 +394,40 @@ align_with_time_bucket(GapFillState *state, Expr *expr)
 	return gapfill_datum_get_internal(value, state->gapfill_typid);
 }
 
-static int64
-get_boundary_expr_value(GapFillState *state, GapFillBoundary boundary, Expr *expr)
+/* Can be called either with PlannerInfo to estimate boundary value at planning time
+ * or with GapFillState to evaluate boundary value at execution time.
+ */
+static Datum
+get_boundary_expr_value(GapFillArgEvalContext *gapfill_ctx, GapFillBoundary boundary, Expr *expr,
+						bool *isnull)
 {
-	Datum arg_value;
-	bool isnull;
-
+	*isnull = false;
+	Datum arg_value = 0;
 	/*
 	 * add an explicit cast here if types do not match
 	 */
-	if (exprType((Node *) expr) != state->gapfill_typid)
+	Oid gapfill_typid = gapfill_ctx->typid;
+	if (exprType((Node *) expr) != gapfill_typid)
 	{
-		Oid cast_oid = get_cast_func(exprType((Node *) expr), state->gapfill_typid);
-
-		expr = (Expr *) makeFuncExpr(cast_oid,
-									 state->gapfill_typid,
-									 list_make1(expr),
-									 InvalidOid,
-									 InvalidOid,
-									 0);
-	}
+		Oid cast_oid = get_cast_func(exprType((Node *) expr), gapfill_typid);
 
-	arg_value = gapfill_exec_expr(state, state->scanslot, expr, &isnull);
+		expr = (Expr *)
+			makeFuncExpr(cast_oid, gapfill_typid, list_make1(expr), InvalidOid, InvalidOid, 0);
+	}
 
-	if (isnull)
+	if (gapfill_ctx->state)
 	{
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("invalid time_bucket_gapfill argument: %s cannot be NULL",
-						boundary == GAPFILL_START ? "start" : "finish"),
-				 errhint("Specify start and finish as arguments or in the WHERE clause.")));
+		GapFillState *state = (GapFillState *) gapfill_ctx->state;
+		arg_value = gapfill_exec_expr(state, state->scanslot, expr, isnull);
+	}
+	else
+	{
+		/* Estimating start/end boundary at planning time:
+		 * bail out with no error if cannot estimate. */
+		arg_value = gapfill_estimate_arg(gapfill_ctx, (Node *) expr, isnull);
 	}
 
-	return gapfill_datum_get_internal(arg_value, state->gapfill_typid);
+	return arg_value;
 }
 
 typedef struct CollectBoundaryContext
@@ -543,15 +539,20 @@ collect_boundary_expressions(Node *node, Var *ts_var)
 	return context.quals;
 }
 
-static int64
-infer_gapfill_boundary(GapFillState *state, GapFillBoundary boundary)
+/* Generic method to collect WHERE quals and obtain suitable start/end gapfill boundaries.
+ * Can be called from the planner with PlannerInfo to estimate boundaries
+ * or with GapFillState to evaluate boundaries at execution time.
+ */
+int64
+infer_gapfill_boundary(GapFillArgEvalContext *gapfill_ctx, GapFillBoundary boundary)
 {
-	CustomScan *cscan = castNode(CustomScan, state->csstate.ss.ps.plan);
-	FuncExpr *func = list_nth(cscan->custom_private, GFP_GapfillFunc);
-	FromExpr *jt = list_nth(cscan->custom_private, GFP_JoinTree);
+	Oid gapfill_typid = gapfill_ctx->typid;
+	List *args = gapfill_ctx->args;
+	FromExpr *jt = gapfill_ctx->jt;
+
 	ListCell *lc;
 	Var *ts_var;
-	TypeCacheEntry *tce = lookup_type_cache(state->gapfill_typid, TYPECACHE_BTREE_OPFAMILY);
+	TypeCacheEntry *tce = lookup_type_cache(gapfill_typid, TYPECACHE_BTREE_OPFAMILY);
 	int strategy;
 	Oid lefttype, righttype;
 	List *quals;
@@ -563,7 +564,7 @@ infer_gapfill_boundary(GapFillState *state, GapFillBoundary boundary)
 	 * if the second argument to time_bucket_gapfill is not a column reference
 	 * we cannot match WHERE clause to the time column
 	 */
-	if (!IsA(lsecond(func->args), Var))
+	if (!IsA(lsecond(args), Var))
 	{
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -572,7 +573,7 @@ infer_gapfill_boundary(GapFillState *state, GapFillBoundary boundary)
 				 errhint("Specify start and finish as arguments or in the WHERE clause.")));
 	}
 
-	ts_var = castNode(Var, lsecond(func->args));
+	ts_var = castNode(Var, lsecond(args));
 
 	quals = collect_boundary_expressions((Node *) jt, ts_var);
 
@@ -613,7 +614,7 @@ infer_gapfill_boundary(GapFillState *state, GapFillBoundary boundary)
 		 * at this stage and Vars will not work either because we execute in
 		 * separate execution context
 		 */
-		if (!is_simple_expr(expr) || !var_equal(ts_var, var))
+		if (!gapfill_is_simple_expr(expr) || !var_equal(ts_var, var))
 		{
 			continue;
 		}
@@ -631,7 +632,35 @@ infer_gapfill_boundary(GapFillState *state, GapFillBoundary boundary)
 			continue;
 		}
 
-		value = get_boundary_expr_value(state, boundary, expr);
+		bool isnull;
+		Datum arg_value = get_boundary_expr_value(gapfill_ctx, boundary, expr, &isnull);
+		/* If boundary is NULL bail out on the rest of the quals */
+		if (isnull)
+		{
+			/* bail out if during execution */
+			if (gapfill_ctx->state)
+			{
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("invalid time_bucket_gapfill argument: %s cannot be NULL",
+								boundary == GAPFILL_START ? "start" : "finish"),
+						 errhint("Specify start and finish as arguments or in the WHERE clause.")));
+			}
+			/* don't estimate the rest of the boundaries but allow planning */
+			else
+			{
+				Assert(gapfill_ctx->root && gapfill_ctx->estimate_failed);
+				return INVALID_ESTIMATE;
+			}
+		}
+		/* Cannot estimate this boundary at planning time: reset and try the next one */
+		if (gapfill_ctx->root && gapfill_ctx->estimate_failed)
+		{
+			gapfill_ctx->estimate_failed = false;
+			continue;
+		}
+
+		value = gapfill_datum_get_internal(arg_value, gapfill_ctx->typid);
 
 		/*
 		 * if the boundary expression operator does not match the operator
@@ -669,6 +698,15 @@ infer_gapfill_boundary(GapFillState *state, GapFillBoundary boundary)
 		return boundary_value;
 	}
 
+	/* Could not estimate any boundaries at planning time */
+	if (gapfill_ctx->root)
+	{
+		gapfill_ctx->estimate_failed = true;
+		return INVALID_ESTIMATE;
+	}
+
+	/* Cannot evaluate boundaries at execution time: bail out */
+	Assert(gapfill_ctx->state);
 	ereport(ERROR,
 			(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 			 errmsg("missing time_bucket_gapfill argument: could not infer %s from WHERE clause",
@@ -677,7 +715,7 @@ infer_gapfill_boundary(GapFillState *state, GapFillBoundary boundary)
 	pg_unreachable();
 }
 
-static Const *
+Const *
 make_const_value_for_gapfill_internal(Oid typid, int64 value)
 {
 	TypeCacheEntry *tce = lookup_type_cache(typid, 0);
@@ -807,7 +845,7 @@ gapfill_initialize_arguments(GapFillState *state)
 	Datum arg_value;
 
 	/* bucket_width */
-	if (!is_simple_expr(linitial(state->args)))
+	if (!gapfill_is_simple_expr(linitial(state->args)))
 	{
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -823,10 +861,10 @@ gapfill_initialize_arguments(GapFillState *state)
 				 errmsg("invalid time_bucket_gapfill argument: bucket_width cannot be NULL")));
 	}
 
-	state->gapfill_period = gapfill_period_get_internal(state->gapfill_typid,
-														exprType(linitial(state->args)),
-														arg_value,
-														&state->gapfill_interval);
+	state->gapfill_period = gapfill_bucket_width_get_internal(state->gapfill_typid,
+															  exprType(linitial(state->args)),
+															  arg_value,
+															  &state->gapfill_interval);
 
 	/*
 	 * this would error when trying to align start and stop to bucket_width as well below
@@ -844,9 +882,19 @@ gapfill_initialize_arguments(GapFillState *state)
 	 * check if gapfill start was left out so we have to infer from WHERE
 	 * clause
 	 */
-	if (is_const_null(get_start_arg(state)))
+	CustomScan *cscan = castNode(CustomScan, state->csstate.ss.ps.plan);
+	GapFillArgEvalContext gapfill_exec_ctx = {
+		.root = NULL,
+		.state = (CustomScanState *) state,
+		.typid = state->gapfill_typid,
+		.args = state->args,
+		.jt = list_nth(cscan->custom_private, GFP_JoinTree),
+		.estimate_failed = false,
+	};
+
+	if (gapfill_is_const_null(get_start_arg(state)))
 	{
-		int64 start = infer_gapfill_boundary(state, GAPFILL_START);
+		int64 start = infer_gapfill_boundary(&gapfill_exec_ctx, GAPFILL_START);
 		Const *expr = make_const_value_for_gapfill_internal(state->gapfill_typid, start);
 
 		state->gapfill_start = align_with_time_bucket(state, (Expr *) expr);
@@ -863,13 +911,13 @@ gapfill_initialize_arguments(GapFillState *state)
 	state->next_offset = state->gapfill_interval;
 
 	/* gap fill end */
-	if (is_const_null(get_finish_arg(state)))
+	if (gapfill_is_const_null(get_finish_arg(state)))
 	{
-		state->gapfill_end = infer_gapfill_boundary(state, GAPFILL_END);
+		state->gapfill_end = infer_gapfill_boundary(&gapfill_exec_ctx, GAPFILL_END);
 	}
 	else
 	{
-		if (!is_simple_expr(get_finish_arg(state)))
+		if (!gapfill_is_simple_expr(get_finish_arg(state)))
 		{
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),

tsl/src/nodes/gapfill/gapfill_internal.h

@@ -126,4 +126,3 @@ typedef struct GapFillState
 Node *gapfill_state_create(CustomScan *);
 Expr *gapfill_adjust_varnos(GapFillState *state, Expr *expr);
 Datum gapfill_exec_expr(GapFillState *state, TupleTableSlot *, Expr *expr, bool *isnull);
-int64 gapfill_datum_get_internal(Datum, Oid);