Skip to content

Commit

Permalink
Fix now() plantime constification with BETWEEN
Browse files Browse the repository at this point in the history
Previously when using BETWEEN ... AND additional constraints in a
WHERE clause, the BETWEEN was not handled correctly because it was
wrapped in a BoolExpr node, which prevented plantime exclusion.
The flattening of such expressions happens in `eval_const_expressions`
which gets called after our constify_now code.
This commit fixes the handling of this case to allow chunk exclusion to
take place at planning time.
Also, makes sure we use our mock timestamp in all places in tests.
Previously we were using a mix of current_timestamp_mock and now(),
which was returning unexpected/incorrect results.
  • Loading branch information
konskov committed Jan 24, 2024
1 parent 92e9009 commit 755ae7a
Show file tree
Hide file tree
Showing 13 changed files with 1,360 additions and 247 deletions.
1 change: 1 addition & 0 deletions .unreleased/PR_6491_now_between
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes: #6491 Enable now() plantime constification with BETWEEN
3 changes: 3 additions & 0 deletions sql/debug_build_utils.sql
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,6 @@ AS '@MODULE_PATHNAME@', 'ts_debug_point_release';

CREATE OR REPLACE FUNCTION debug_waitpoint_id(TEXT) RETURNS BIGINT LANGUAGE C VOLATILE STRICT
AS '@MODULE_PATHNAME@', 'ts_debug_point_id';

CREATE OR REPLACE FUNCTION ts_now_mock()
RETURNS TIMESTAMPTZ AS '@MODULE_PATHNAME@', 'ts_now_mock' LANGUAGE C STABLE STRICT PARALLEL SAFE;
12 changes: 3 additions & 9 deletions src/planner/constify_now.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ get_hypertable_dimension(Oid relid, int flags)
return hyperspace_get_open_dimension(ht->space, 0);
}

static bool
bool
is_valid_now_func(Node *node)
{
if (IsA(node, FuncExpr) && castNode(FuncExpr, node)->funcid == F_NOW)
Expand Down Expand Up @@ -249,17 +249,11 @@ ts_constify_now(PlannerInfo *root, List *rtable, Node *node)

foreach (lc, be->args)
{
if (IsA(lfirst(lc), OpExpr) && is_valid_now_expr(lfirst_node(OpExpr, lc), rtable))
{
OpExpr *op = lfirst_node(OpExpr, lc);
additions = lappend(additions, constify_now_expr(root, op));
}
additions = lappend(additions, ts_constify_now(root, rtable, (Node *) lfirst(lc)));
}

if (additions)
{
be->args = list_concat(be->args, additions);
}
be->args = additions;

break;
}
Expand Down
56 changes: 56 additions & 0 deletions src/planner/planner.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include <optimizer/plancat.h>
#include <parser/analyze.h>
#include <tcop/tcopprot.h>
#include <utils/fmgrprotos.h>
#include "compat/compat-msvc-exit.h"

#include <math.h>
Expand Down Expand Up @@ -279,6 +280,47 @@ typedef struct
PlannerInfo *root;
} PreprocessQueryContext;

void
replace_now_mock_walker(PlannerInfo *root, Node *clause, Oid funcid)
{
/* whenever we encounter a FuncExpr with now(), replace it with the supplied funcid */
switch (nodeTag(clause))
{
case T_FuncExpr:
{
if (is_valid_now_func(clause))
{
FuncExpr *fe = castNode(FuncExpr, clause);
fe->funcid = funcid;
return;
}
break;
}
case T_OpExpr:
{
ListCell *lc;
OpExpr *oe = castNode(OpExpr, clause);
foreach (lc, oe->args)
{
replace_now_mock_walker(root, (Node *) lfirst(lc), funcid);
}
break;
}
case T_BoolExpr:
{
ListCell *lc;
BoolExpr *be = castNode(BoolExpr, clause);
foreach (lc, be->args)
{
replace_now_mock_walker(root, (Node *) lfirst(lc), funcid);
}
break;
}
default:
return;
}
}

/*
* Preprocess the query tree, including, e.g., subqueries.
*
Expand Down Expand Up @@ -310,6 +352,20 @@ preprocess_query(Node *node, PreprocessQueryContext *context)
{
from->quals =
ts_constify_now(context->root, context->current_query->rtable, from->quals);
#ifdef TS_DEBUG
/*
* only replace if GUC is also set. This is used for testing purposes only,
* so no need to change the output for other tests in DEBUG builds
*/
if (ts_current_timestamp_mock != NULL && strlen(ts_current_timestamp_mock) != 0)
{
Oid funcid_mock;
const char *funcname = "ts_now_mock()";
funcid_mock = DatumGetObjectId(
DirectFunctionCall1(regprocedurein, CStringGetDatum(funcname)));
replace_now_mock_walker(context->root, from->quals, funcid_mock);
}
#endif
}
/*
* We only amend space constraints for UPDATE/DELETE and SELECT FOR UPDATE
Expand Down
21 changes: 21 additions & 0 deletions src/time_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -567,3 +567,24 @@ ts_get_mock_time_or_current_time(void)
return res;
}
#endif

TS_FUNCTION_INFO_V1(ts_now_mock);

/* return mock time for testing */
Datum
ts_now_mock(PG_FUNCTION_ARGS)
{
Datum res;
#ifdef TS_DEBUG
if (ts_current_timestamp_mock != NULL && strlen(ts_current_timestamp_mock) != 0)
{
res = DirectFunctionCall3(timestamptz_in,
CStringGetDatum(ts_current_timestamp_mock),
0,
Int32GetDatum(-1));
return res;
}
#endif
res = TimestampTzGetDatum(GetCurrentTimestamp());
return res;

Check warning on line 589 in src/time_utils.c

View check run for this annotation

Codecov / codecov/patch

src/time_utils.c#L588-L589

Added lines #L588 - L589 were not covered by tests
}
2 changes: 2 additions & 0 deletions src/time_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,5 @@ extern TSDLLEXPORT int64 ts_subtract_integer_from_now_saturating(Oid now_func, i
#ifdef TS_DEBUG
extern TSDLLEXPORT Datum ts_get_mock_time_or_current_time(void);
#endif

bool is_valid_now_func(Node *node);
2 changes: 2 additions & 0 deletions src/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -246,3 +246,5 @@ ts_get_relation_relid(char const *schema_name, char const *relation_name, bool r
return InvalidOid;
}
}

void replace_now_mock_walker(PlannerInfo *root, Node *clause, Oid funcid);
Loading

0 comments on commit 755ae7a

Please sign in to comment.