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, make 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 22, 2024
1 parent e89bc24 commit cb6e424
Show file tree
Hide file tree
Showing 14 changed files with 1,363 additions and 248 deletions.
3 changes: 3 additions & 0 deletions sql/updates/latest-dev.sql
Original file line number Diff line number Diff line change
Expand Up @@ -397,3 +397,6 @@ CREATE FUNCTION _timescaledb_functions.constraint_clone(constraint_oid OID,targe
DROP FUNCTION IF EXISTS _timescaledb_functions.chunks_in;
DROP FUNCTION IF EXISTS _timescaledb_internal.chunks_in;


CREATE FUNCTION _timescaledb_functions.ts_replace_now_mock()
RETURNS TIMESTAMPTZ AS '@MODULE_PATHNAME@', 'ts_replace_now_mock' LANGUAGE C STABLE STRICT PARALLEL SAFE;
1 change: 1 addition & 0 deletions sql/updates/reverse-dev.sql
Original file line number Diff line number Diff line change
Expand Up @@ -770,3 +770,4 @@ AS 'BEGIN END' LANGUAGE PLPGSQL SET search_path TO pg_catalog,pg_temp;

DROP FUNCTION IF EXISTS _timescaledb_functions.get_orderby_defaults(regclass,text[]);
DROP FUNCTION IF EXISTS _timescaledb_functions.get_segmentby_defaults(regclass);
DROP FUNCTION IF EXISTS _timescaledb_functions.ts_replace_now_mock();
3 changes: 3 additions & 0 deletions sql/util_time.sql
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,6 @@ RETURNS INT8 AS '@MODULE_PATHNAME@', 'ts_continuous_agg_watermark_materialized'

CREATE OR REPLACE FUNCTION _timescaledb_functions.subtract_integer_from_now( hypertable_relid REGCLASS, lag INT8 )
RETURNS INT8 AS '@MODULE_PATHNAME@', 'ts_subtract_integer_from_now' LANGUAGE C STABLE STRICT;

CREATE OR REPLACE FUNCTION _timescaledb_functions.ts_replace_now_mock()
RETURNS TIMESTAMPTZ AS '@MODULE_PATHNAME@', 'ts_replace_now_mock' LANGUAGE C STABLE STRICT PARALLEL SAFE;
13 changes: 3 additions & 10 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 @@ -147,7 +147,6 @@ make_now_const()
FLOAT8PASSBYVAL);
}

/* returns a copy of the expression with the now() call constified */
/*
* op will be OpExpr with Var > now() - Expr
*/
Expand Down Expand Up @@ -249,17 +248,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; // keep compiler happy
}
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 = "_timescaledb_functions.ts_replace_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_replace_now_mock);

/* return mock time for testing */
Datum
ts_replace_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 cb6e424

Please sign in to comment.