From cb81c331ae4a9fa233106439ef5f73c02b702065 Mon Sep 17 00:00:00 2001 From: Konstantina Skovola Date: Tue, 28 Mar 2023 16:27:00 +0300 Subject: [PATCH] Allow named time_bucket arguments in Cagg definition Fixes #5450 --- CHANGELOG.md | 1 + tsl/src/continuous_aggs/create.c | 39 +++++++++++++++++++----- tsl/test/expected/cagg_ddl.out | 30 ++++++++++++++++++ tsl/test/expected/cagg_ddl_dist_ht.out | 30 ++++++++++++++++++ tsl/test/sql/include/cagg_ddl_common.sql | 31 +++++++++++++++++++ 5 files changed, 124 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b079a132c..51921b802 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ accidentally triggering the load of a previous DB version.** * #5470 Ensure superuser perms during copy/move chunk * #5459 Fix issue creating dimensional constraints * #5499 Do not segfault on large histogram() parameters +* #5497 Allow named time_bucket arguments in Cagg definition **Thanks** * @nikolaps for reporting an issue with the COPY fetcher diff --git a/tsl/src/continuous_aggs/create.c b/tsl/src/continuous_aggs/create.c index 12330b357..b9d23a030 100644 --- a/tsl/src/continuous_aggs/create.c +++ b/tsl/src/continuous_aggs/create.c @@ -764,6 +764,7 @@ caggtimebucket_validate(CAggTimebucketInfo *tbinfo, List *groupClause, List *tar ListCell *l; bool found = false; bool custom_origin = false; + Const *const_arg; /* Make sure tbinfo was initialized. This assumption is used below. */ Assert(tbinfo->bucket_width == 0); @@ -804,6 +805,9 @@ caggtimebucket_validate(CAggTimebucketInfo *tbinfo, List *groupClause, List *tar /* Only column allowed : time_bucket('1day', ) */ col_arg = lsecond(fe->args); + /* Could be a named argument */ + if (IsA(col_arg, NamedArgExpr)) + col_arg = (Node *) castNode(NamedArgExpr, col_arg)->arg; if (!(IsA(col_arg, Var)) || ((Var *) col_arg)->varattno != tbinfo->htpartcolno) ereport(ERROR, @@ -831,6 +835,7 @@ caggtimebucket_validate(CAggTimebucketInfo *tbinfo, List *groupClause, List *tar if (list_length(fe->args) >= 4) { + /* origin */ Const *arg = check_time_bucket_argument(lfourth(fe->args), "fourth"); if (exprType((Node *) arg) == TEXTOID) { @@ -854,19 +859,22 @@ caggtimebucket_validate(CAggTimebucketInfo *tbinfo, List *groupClause, List *tar /* Origin is always 3rd arg for date variants. */ if (list_length(fe->args) == 3) { + Node *arg = lthird(fe->args); custom_origin = true; + /* this function also takes care of named arguments */ + const_arg = check_time_bucket_argument(arg, "third"); tbinfo->origin = DatumGetTimestamp( - DirectFunctionCall1(date_timestamp, - castNode(Const, lthird(fe->args))->constvalue)); + DirectFunctionCall1(date_timestamp, const_arg->constvalue)); } break; case TIMESTAMPOID: /* Origin is always 3rd arg for timestamp variants. */ if (list_length(fe->args) == 3) { + Node *arg = lthird(fe->args); custom_origin = true; - tbinfo->origin = - DatumGetTimestamp(castNode(Const, lthird(fe->args))->constvalue); + const_arg = check_time_bucket_argument(arg, "third"); + tbinfo->origin = DatumGetTimestamp(const_arg->constvalue); } break; case TIMESTAMPTZOID: @@ -881,8 +889,20 @@ caggtimebucket_validate(CAggTimebucketInfo *tbinfo, List *groupClause, List *tar exprType(lfourth(fe->args)) == TIMESTAMPTZOID) { custom_origin = true; - tbinfo->origin = - DatumGetTimestampTz(castNode(Const, lfourth(fe->args))->constvalue); + if (IsA(lfourth(fe->args), Const)) + { + tbinfo->origin = + DatumGetTimestampTz(castNode(Const, lfourth(fe->args))->constvalue); + } + /* could happen in a statement like time_bucket('1h', .., 'utc', origin => + * ...) */ + else if (IsA(lfourth(fe->args), NamedArgExpr)) + { + Const *constval = + check_time_bucket_argument(lfourth(fe->args), "fourth"); + + tbinfo->origin = DatumGetTimestampTz(constval->constvalue); + } } } if (custom_origin && TIMESTAMP_NOT_FINITE(tbinfo->origin)) @@ -898,7 +918,12 @@ caggtimebucket_validate(CAggTimebucketInfo *tbinfo, List *groupClause, List *tar * partitioning column as int constants default to int4 and so expression would * have a cast and not be a Const. */ - width_arg = eval_const_expressions(NULL, linitial(fe->args)); + width_arg = linitial(fe->args); + + if (IsA(width_arg, NamedArgExpr)) + width_arg = (Node *) castNode(NamedArgExpr, width_arg)->arg; + + width_arg = eval_const_expressions(NULL, width_arg); if (IsA(width_arg, Const)) { Const *width = castNode(Const, width_arg); diff --git a/tsl/test/expected/cagg_ddl.out b/tsl/test/expected/cagg_ddl.out index 5678f4bf7..c1597d644 100644 --- a/tsl/test/expected/cagg_ddl.out +++ b/tsl/test/expected/cagg_ddl.out @@ -1939,3 +1939,33 @@ SELECT * FROM cashflows; Thu Nov 01 17:00:00 2018 PDT | 1 | 10 | 11 (6 rows) +-- test cagg creation with named arguments in time_bucket +-- note that positional arguments cannot follow named arguments +-- 1. test named origin +-- 2. test named timezone +-- 3. test named ts +-- 4. test named bucket width +-- named origin +CREATE MATERIALIZED VIEW cagg_named_origin WITH +(timescaledb.continuous) AS +SELECT time_bucket('1h', time, 'UTC', origin => '2001-01-03 01:23:45') AS bucket, +avg(amount) as avg_amount +FROM transactions GROUP BY 1 WITH NO DATA; +-- named timezone +CREATE MATERIALIZED VIEW cagg_named_tz_origin WITH +(timescaledb.continuous) AS +SELECT time_bucket('1h', time, timezone => 'UTC', origin => '2001-01-03 01:23:45') AS bucket, +avg(amount) as avg_amount +FROM transactions GROUP BY 1 WITH NO DATA; +-- named ts +CREATE MATERIALIZED VIEW cagg_named_ts_tz_origin WITH +(timescaledb.continuous) AS +SELECT time_bucket('1h', ts => time, timezone => 'UTC', origin => '2001-01-03 01:23:45') AS bucket, +avg(amount) as avg_amount +FROM transactions GROUP BY 1 WITH NO DATA; +-- named bucket width +CREATE MATERIALIZED VIEW cagg_named_all WITH +(timescaledb.continuous) AS +SELECT time_bucket(bucket_width => '1h', ts => time, timezone => 'UTC', origin => '2001-01-03 01:23:45') AS bucket, +avg(amount) as avg_amount +FROM transactions GROUP BY 1 WITH NO DATA; diff --git a/tsl/test/expected/cagg_ddl_dist_ht.out b/tsl/test/expected/cagg_ddl_dist_ht.out index 4a3355bca..4bebcb8bb 100644 --- a/tsl/test/expected/cagg_ddl_dist_ht.out +++ b/tsl/test/expected/cagg_ddl_dist_ht.out @@ -1982,6 +1982,36 @@ SELECT * FROM cashflows; Thu Nov 01 17:00:00 2018 PDT | 1 | 10 | 11 (6 rows) +-- test cagg creation with named arguments in time_bucket +-- note that positional arguments cannot follow named arguments +-- 1. test named origin +-- 2. test named timezone +-- 3. test named ts +-- 4. test named bucket width +-- named origin +CREATE MATERIALIZED VIEW cagg_named_origin WITH +(timescaledb.continuous) AS +SELECT time_bucket('1h', time, 'UTC', origin => '2001-01-03 01:23:45') AS bucket, +avg(amount) as avg_amount +FROM transactions GROUP BY 1 WITH NO DATA; +-- named timezone +CREATE MATERIALIZED VIEW cagg_named_tz_origin WITH +(timescaledb.continuous) AS +SELECT time_bucket('1h', time, timezone => 'UTC', origin => '2001-01-03 01:23:45') AS bucket, +avg(amount) as avg_amount +FROM transactions GROUP BY 1 WITH NO DATA; +-- named ts +CREATE MATERIALIZED VIEW cagg_named_ts_tz_origin WITH +(timescaledb.continuous) AS +SELECT time_bucket('1h', ts => time, timezone => 'UTC', origin => '2001-01-03 01:23:45') AS bucket, +avg(amount) as avg_amount +FROM transactions GROUP BY 1 WITH NO DATA; +-- named bucket width +CREATE MATERIALIZED VIEW cagg_named_all WITH +(timescaledb.continuous) AS +SELECT time_bucket(bucket_width => '1h', ts => time, timezone => 'UTC', origin => '2001-01-03 01:23:45') AS bucket, +avg(amount) as avg_amount +FROM transactions GROUP BY 1 WITH NO DATA; -- cleanup \c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER; DROP DATABASE :DATA_NODE_1; diff --git a/tsl/test/sql/include/cagg_ddl_common.sql b/tsl/test/sql/include/cagg_ddl_common.sql index cdf8db465..b81fdb662 100644 --- a/tsl/test/sql/include/cagg_ddl_common.sql +++ b/tsl/test/sql/include/cagg_ddl_common.sql @@ -1282,3 +1282,34 @@ WHERE user_view_name = 'cashflows' \d+ 'cashflows' SELECT * FROM cashflows; + +-- test cagg creation with named arguments in time_bucket +-- note that positional arguments cannot follow named arguments +-- 1. test named origin +-- 2. test named timezone +-- 3. test named ts +-- 4. test named bucket width +-- named origin +CREATE MATERIALIZED VIEW cagg_named_origin WITH +(timescaledb.continuous) AS +SELECT time_bucket('1h', time, 'UTC', origin => '2001-01-03 01:23:45') AS bucket, +avg(amount) as avg_amount +FROM transactions GROUP BY 1 WITH NO DATA; +-- named timezone +CREATE MATERIALIZED VIEW cagg_named_tz_origin WITH +(timescaledb.continuous) AS +SELECT time_bucket('1h', time, timezone => 'UTC', origin => '2001-01-03 01:23:45') AS bucket, +avg(amount) as avg_amount +FROM transactions GROUP BY 1 WITH NO DATA; +-- named ts +CREATE MATERIALIZED VIEW cagg_named_ts_tz_origin WITH +(timescaledb.continuous) AS +SELECT time_bucket('1h', ts => time, timezone => 'UTC', origin => '2001-01-03 01:23:45') AS bucket, +avg(amount) as avg_amount +FROM transactions GROUP BY 1 WITH NO DATA; +-- named bucket width +CREATE MATERIALIZED VIEW cagg_named_all WITH +(timescaledb.continuous) AS +SELECT time_bucket(bucket_width => '1h', ts => time, timezone => 'UTC', origin => '2001-01-03 01:23:45') AS bucket, +avg(amount) as avg_amount +FROM transactions GROUP BY 1 WITH NO DATA;