Skip to content

Commit 5df1f04

Browse files
Improve gapfill row count estimate
1 parent 11b0fd1 commit 5df1f04

11 files changed

Lines changed: 1822 additions & 53 deletions

File tree

.unreleased/pr_9735

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Implements: #9735 Improve GapFill row count estimate
2+
Thanks: @Fabian-2596 for suggesting more accurate GapFill row count estimate

tsl/src/nodes/gapfill/gapfill.h

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <postgres.h>
99
#include <nodes/pathnodes.h>
1010
#include <nodes/primnodes.h>
11+
#include <utils/builtins.h>
1112

1213
#define GAPFILL_FUNCTION "time_bucket_gapfill"
1314
#define GAPFILL_LOCF_FUNCTION "locf"
@@ -34,3 +35,29 @@ typedef struct GapFillPath
3435
CustomPath cpath;
3536
FuncExpr *func; /* time_bucket_gapfill function call */
3637
} GapFillPath;
38+
39+
typedef enum GapFillBoundary
40+
{
41+
GAPFILL_START,
42+
GAPFILL_END,
43+
} GapFillBoundary;
44+
45+
typedef struct GapFillArgEvalContext
46+
{
47+
PlannerInfo *root; /* needed for evaluation at planning time */
48+
CustomScanState *state; /* needed for evaluation at execution time */
49+
Oid typid; /* gapfill function type */
50+
List *args; /* gapfill function arguments */
51+
FromExpr *jt; /* needed for inferring boundaries from WHERE */
52+
bool estimate_failed; /* whether estimation during planning failed */
53+
} GapFillArgEvalContext;
54+
55+
extern int64 infer_gapfill_boundary(GapFillArgEvalContext *gapfill_ctx, GapFillBoundary boundary);
56+
extern Datum gapfill_estimate_arg(GapFillArgEvalContext *gapfill_plan_ctx, Node *arg);
57+
58+
extern bool gapfill_is_const_null(Expr *expr);
59+
extern bool gapfill_is_simple_expr(Expr *node);
60+
extern Const *make_const_value_for_gapfill_internal(Oid typid, int64 value);
61+
extern int64 gapfill_datum_get_internal(Datum, Oid);
62+
extern int64 gapfill_bucket_width_get_internal(Oid timetype, Oid argtype, Datum arg,
63+
Interval **interval);

tsl/src/nodes/gapfill/gapfill_exec.c

Lines changed: 86 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,14 @@
2828
#include <utils/typcache.h>
2929

3030
#include <compat/compat.h>
31+
#include "estimate.h"
3132
#include "gapfill.h"
3233
#include "gapfill_internal.h"
3334
#include "interpolate.h"
3435
#include "locf.h"
3536
#include "time_bucket.h"
3637
#include <annotations.h>
3738

38-
typedef enum GapFillBoundary
39-
{
40-
GAPFILL_START,
41-
GAPFILL_END,
42-
} GapFillBoundary;
43-
4439
typedef union GapFillColumnStateUnion
4540
{
4641
GapFillColumnState *base;
@@ -175,8 +170,8 @@ get_timezone_arg(GapFillState *state)
175170
return lthird(state->args);
176171
}
177172

178-
static inline int64
179-
gapfill_period_get_internal(Oid timetype, Oid argtype, Datum arg, Interval **interval)
173+
int64
174+
gapfill_bucket_width_get_internal(Oid timetype, Oid argtype, Datum arg, Interval **interval)
180175
{
181176
switch (timetype)
182177
{
@@ -240,8 +235,8 @@ gapfill_state_create(CustomScan *cscan)
240235
return (Node *) state;
241236
}
242237

243-
static bool
244-
is_const_null(Expr *expr)
238+
bool
239+
gapfill_is_const_null(Expr *expr)
245240
{
246241
return IsA(expr, Const) && castNode(Const, expr)->constisnull;
247242
}
@@ -288,7 +283,7 @@ var_equal(Var *v1, Var *v2)
288283
}
289284

290285
static bool
291-
is_simple_expr_walker(Node *node, void *context)
286+
gapfill_is_simple_expr_walker(Node *node, void *context)
292287
{
293288
if (node == NULL)
294289
{
@@ -328,22 +323,22 @@ is_simple_expr_walker(Node *node, void *context)
328323
default:
329324
return true;
330325
}
331-
return expression_tree_walker(node, is_simple_expr_walker, context);
326+
return expression_tree_walker(node, gapfill_is_simple_expr_walker, context);
332327
}
333328

334329
/*
335330
* check if expression is simple expression and contains only simple
336331
* subexpressions
337332
*/
338-
static bool
339-
is_simple_expr(Expr *node)
333+
bool
334+
gapfill_is_simple_expr(Expr *node)
340335
{
341336
/*
342337
* since expression_tree_walker does early exit on true and we use that to
343338
* skip processing on first non-simple expression we invert return value
344339
* from expression_tree_walker here
345340
*/
346-
return !is_simple_expr_walker((Node *) node, NULL);
341+
return !gapfill_is_simple_expr_walker((Node *) node, NULL);
347342
}
348343

349344
/*
@@ -360,7 +355,7 @@ align_with_time_bucket(GapFillState *state, Expr *expr)
360355
Datum value;
361356
bool isnull;
362357

363-
if (!is_simple_expr(expr))
358+
if (!gapfill_is_simple_expr(expr))
364359
{
365360
ereport(ERROR,
366361
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -399,28 +394,42 @@ align_with_time_bucket(GapFillState *state, Expr *expr)
399394
return gapfill_datum_get_internal(value, state->gapfill_typid);
400395
}
401396

397+
/* Can be called either with PlannerInfo to estimate boundary value at planning time
398+
* or with GapFillState to evaluate boundary value at execution time.
399+
*/
402400
static int64
403-
get_boundary_expr_value(GapFillState *state, GapFillBoundary boundary, Expr *expr)
401+
get_boundary_expr_value(GapFillArgEvalContext *gapfill_ctx, GapFillBoundary boundary, Expr *expr)
404402
{
405-
Datum arg_value;
406-
bool isnull;
403+
Oid gapfill_typid = gapfill_ctx->typid;
407404

405+
Datum arg_value;
406+
bool isnull = false;
408407
/*
409408
* add an explicit cast here if types do not match
410409
*/
411-
if (exprType((Node *) expr) != state->gapfill_typid)
410+
if (exprType((Node *) expr) != gapfill_typid)
412411
{
413-
Oid cast_oid = get_cast_func(exprType((Node *) expr), state->gapfill_typid);
414-
415-
expr = (Expr *) makeFuncExpr(cast_oid,
416-
state->gapfill_typid,
417-
list_make1(expr),
418-
InvalidOid,
419-
InvalidOid,
420-
0);
412+
Oid cast_oid = get_cast_func(exprType((Node *) expr), gapfill_typid);
413+
414+
expr = (Expr *)
415+
makeFuncExpr(cast_oid, gapfill_typid, list_make1(expr), InvalidOid, InvalidOid, 0);
421416
}
422417

423-
arg_value = gapfill_exec_expr(state, state->scanslot, expr, &isnull);
418+
if (gapfill_ctx->state)
419+
{
420+
GapFillState *state = (GapFillState *) gapfill_ctx->state;
421+
arg_value = gapfill_exec_expr(state, state->scanslot, expr, &isnull);
422+
}
423+
else
424+
{
425+
/* Estimating start/end boundary at planning time:
426+
* bail out with no error if cannot estimate. */
427+
arg_value = gapfill_estimate_arg(gapfill_ctx, (Node *) expr);
428+
if (gapfill_ctx->estimate_failed)
429+
{
430+
return INVALID_ESTIMATE;
431+
}
432+
}
424433

425434
if (isnull)
426435
{
@@ -431,7 +440,7 @@ get_boundary_expr_value(GapFillState *state, GapFillBoundary boundary, Expr *exp
431440
errhint("Specify start and finish as arguments or in the WHERE clause.")));
432441
}
433442

434-
return gapfill_datum_get_internal(arg_value, state->gapfill_typid);
443+
return gapfill_datum_get_internal(arg_value, gapfill_typid);
435444
}
436445

437446
typedef struct CollectBoundaryContext
@@ -543,15 +552,20 @@ collect_boundary_expressions(Node *node, Var *ts_var)
543552
return context.quals;
544553
}
545554

546-
static int64
547-
infer_gapfill_boundary(GapFillState *state, GapFillBoundary boundary)
555+
/* Generic method to collect WHERE quals and obtain suitable start/end gapfill boundaries.
556+
* Can be called from the planner with PlannerInfo to estimate boundaries
557+
* or with GapFillState to evaluate boundaries at execution time.
558+
*/
559+
int64
560+
infer_gapfill_boundary(GapFillArgEvalContext *gapfill_ctx, GapFillBoundary boundary)
548561
{
549-
CustomScan *cscan = castNode(CustomScan, state->csstate.ss.ps.plan);
550-
FuncExpr *func = list_nth(cscan->custom_private, GFP_GapfillFunc);
551-
FromExpr *jt = list_nth(cscan->custom_private, GFP_JoinTree);
562+
Oid gapfill_typid = gapfill_ctx->typid;
563+
List *args = gapfill_ctx->args;
564+
FromExpr *jt = gapfill_ctx->jt;
565+
552566
ListCell *lc;
553567
Var *ts_var;
554-
TypeCacheEntry *tce = lookup_type_cache(state->gapfill_typid, TYPECACHE_BTREE_OPFAMILY);
568+
TypeCacheEntry *tce = lookup_type_cache(gapfill_typid, TYPECACHE_BTREE_OPFAMILY);
555569
int strategy;
556570
Oid lefttype, righttype;
557571
List *quals;
@@ -563,7 +577,7 @@ infer_gapfill_boundary(GapFillState *state, GapFillBoundary boundary)
563577
* if the second argument to time_bucket_gapfill is not a column reference
564578
* we cannot match WHERE clause to the time column
565579
*/
566-
if (!IsA(lsecond(func->args), Var))
580+
if (!IsA(lsecond(args), Var))
567581
{
568582
ereport(ERROR,
569583
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -572,7 +586,7 @@ infer_gapfill_boundary(GapFillState *state, GapFillBoundary boundary)
572586
errhint("Specify start and finish as arguments or in the WHERE clause.")));
573587
}
574588

575-
ts_var = castNode(Var, lsecond(func->args));
589+
ts_var = castNode(Var, lsecond(args));
576590

577591
quals = collect_boundary_expressions((Node *) jt, ts_var);
578592

@@ -613,7 +627,7 @@ infer_gapfill_boundary(GapFillState *state, GapFillBoundary boundary)
613627
* at this stage and Vars will not work either because we execute in
614628
* separate execution context
615629
*/
616-
if (!is_simple_expr(expr) || !var_equal(ts_var, var))
630+
if (!gapfill_is_simple_expr(expr) || !var_equal(ts_var, var))
617631
{
618632
continue;
619633
}
@@ -631,7 +645,11 @@ infer_gapfill_boundary(GapFillState *state, GapFillBoundary boundary)
631645
continue;
632646
}
633647

634-
value = get_boundary_expr_value(state, boundary, expr);
648+
value = get_boundary_expr_value(gapfill_ctx, boundary, expr);
649+
if (gapfill_ctx->estimate_failed)
650+
{
651+
return INVALID_ESTIMATE;
652+
}
635653

636654
/*
637655
* if the boundary expression operator does not match the operator
@@ -669,6 +687,13 @@ infer_gapfill_boundary(GapFillState *state, GapFillBoundary boundary)
669687
return boundary_value;
670688
}
671689

690+
/* called at planning time: cannot estimate but can still plan */
691+
if (gapfill_ctx->root)
692+
{
693+
gapfill_ctx->estimate_failed = true;
694+
return INVALID_ESTIMATE;
695+
}
696+
672697
ereport(ERROR,
673698
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
674699
errmsg("missing time_bucket_gapfill argument: could not infer %s from WHERE clause",
@@ -677,7 +702,7 @@ infer_gapfill_boundary(GapFillState *state, GapFillBoundary boundary)
677702
pg_unreachable();
678703
}
679704

680-
static Const *
705+
Const *
681706
make_const_value_for_gapfill_internal(Oid typid, int64 value)
682707
{
683708
TypeCacheEntry *tce = lookup_type_cache(typid, 0);
@@ -807,7 +832,7 @@ gapfill_initialize_arguments(GapFillState *state)
807832
Datum arg_value;
808833

809834
/* bucket_width */
810-
if (!is_simple_expr(linitial(state->args)))
835+
if (!gapfill_is_simple_expr(linitial(state->args)))
811836
{
812837
ereport(ERROR,
813838
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -823,10 +848,10 @@ gapfill_initialize_arguments(GapFillState *state)
823848
errmsg("invalid time_bucket_gapfill argument: bucket_width cannot be NULL")));
824849
}
825850

826-
state->gapfill_period = gapfill_period_get_internal(state->gapfill_typid,
827-
exprType(linitial(state->args)),
828-
arg_value,
829-
&state->gapfill_interval);
851+
state->gapfill_period = gapfill_bucket_width_get_internal(state->gapfill_typid,
852+
exprType(linitial(state->args)),
853+
arg_value,
854+
&state->gapfill_interval);
830855

831856
/*
832857
* this would error when trying to align start and stop to bucket_width as well below
@@ -844,9 +869,19 @@ gapfill_initialize_arguments(GapFillState *state)
844869
* check if gapfill start was left out so we have to infer from WHERE
845870
* clause
846871
*/
847-
if (is_const_null(get_start_arg(state)))
872+
CustomScan *cscan = castNode(CustomScan, state->csstate.ss.ps.plan);
873+
GapFillArgEvalContext gapfill_exec_ctx = {
874+
.root = NULL,
875+
.state = (CustomScanState *) state,
876+
.typid = state->gapfill_typid,
877+
.args = state->args,
878+
.jt = list_nth(cscan->custom_private, GFP_JoinTree),
879+
.estimate_failed = false,
880+
};
881+
882+
if (gapfill_is_const_null(get_start_arg(state)))
848883
{
849-
int64 start = infer_gapfill_boundary(state, GAPFILL_START);
884+
int64 start = infer_gapfill_boundary(&gapfill_exec_ctx, GAPFILL_START);
850885
Const *expr = make_const_value_for_gapfill_internal(state->gapfill_typid, start);
851886

852887
state->gapfill_start = align_with_time_bucket(state, (Expr *) expr);
@@ -863,13 +898,13 @@ gapfill_initialize_arguments(GapFillState *state)
863898
state->next_offset = state->gapfill_interval;
864899

865900
/* gap fill end */
866-
if (is_const_null(get_finish_arg(state)))
901+
if (gapfill_is_const_null(get_finish_arg(state)))
867902
{
868-
state->gapfill_end = infer_gapfill_boundary(state, GAPFILL_END);
903+
state->gapfill_end = infer_gapfill_boundary(&gapfill_exec_ctx, GAPFILL_END);
869904
}
870905
else
871906
{
872-
if (!is_simple_expr(get_finish_arg(state)))
907+
if (!gapfill_is_simple_expr(get_finish_arg(state)))
873908
{
874909
ereport(ERROR,
875910
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),

tsl/src/nodes/gapfill/gapfill_internal.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,4 +126,3 @@ typedef struct GapFillState
126126
Node *gapfill_state_create(CustomScan *);
127127
Expr *gapfill_adjust_varnos(GapFillState *state, Expr *expr);
128128
Datum gapfill_exec_expr(GapFillState *state, TupleTableSlot *, Expr *expr, bool *isnull);
129-
int64 gapfill_datum_get_internal(Datum, Oid);

0 commit comments

Comments
 (0)