Skip to content

Commit

Permalink
Fix segfault on CAggs with multiple JOINs
Browse files Browse the repository at this point in the history 
Creating or changing to realtime a Continuous Aggregate with multiple
joins was leading to a segfault.

Fixed it by dealing properly with the `varno` when creating the `Quals`
for the union view in realtime mode.

Also get rid of some left over when we relaxed the CAggs joint
restrictions in timescale#7111
  • Loading branch information
@fabriziomello
fabriziomello committed Jul 22, 2024
1 parent d18361e commit 971c6ed
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 52 deletions.
52 changes: 10 additions & 42 deletions tsl/src/continuous_aggs/common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1332,55 +1332,23 @@ build_union_query(CAggTimebucketInfo *tbinfo, int matpartcolno, Query *q1, Query
/*
* If there is join in CAgg definition then adjust varno
* to get time column from the hypertable in the join.
*
* In case of joins it is enough to check if the first node is not RangeTblRef,
* because the jointree has RangeTblRef as leaves and JoinExpr above them.
* So if JoinExpr is present, it is the first node.
* Other cases of join i.e. without explicit JOIN clause is confirmed
* by reading the length of rtable.
*/
if (list_length(q2->rtable) == CONTINUOUS_AGG_MAX_JOIN_RELATIONS ||
!IsA(linitial(q2->jointree->fromlist), RangeTblRef))
varno = list_length(q2->rtable);
int nvarno = 1;
foreach (lc2, q2->rtable)
{
Oid normal_table_id = InvalidOid;
RangeTblEntry *rte = NULL;
RangeTblEntry *rte_other = NULL;

if (list_length(q2->rtable) == CONTINUOUS_AGG_MAX_JOIN_RELATIONS)
{
RangeTblRef *rtref = linitial_node(RangeTblRef, q2->jointree->fromlist);
rte = list_nth(q2->rtable, rtref->rtindex - 1);
RangeTblRef *rtref_other = lsecond_node(RangeTblRef, q2->jointree->fromlist);
rte_other = list_nth(q2->rtable, rtref_other->rtindex - 1);
}
else if (!IsA(linitial(q2->jointree->fromlist), RangeTblRef))
RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc2);
if (rte->rtekind == RTE_RELATION)
{
ListCell *l;
foreach (l, q2->jointree->fromlist)
/* look for hypertable in the list of RangeTableEntry */
if (rte->relid == tbinfo->htoid)
{
Node *jtnode = (Node *) lfirst(l);
JoinExpr *join = NULL;
if (IsA(jtnode, JoinExpr))
{
join = castNode(JoinExpr, jtnode);
rte = list_nth(q2->rtable, ((RangeTblRef *) join->larg)->rtindex - 1);
rte_other = list_nth(q2->rtable, ((RangeTblRef *) join->rarg)->rtindex - 1);
}
varno = nvarno;
break;
}
}
if (rte->relkind == RELKIND_VIEW)
normal_table_id = rte_other->relid;
else if (rte_other->relkind == RELKIND_VIEW)
normal_table_id = rte->relid;
else
normal_table_id = ts_is_hypertable(rte->relid) ? rte_other->relid : rte->relid;
if (normal_table_id == rte->relid)
varno = 2;
else
varno = 1;
nvarno++;
}
else
varno = list_length(q2->rtable);

q2_quals = build_union_query_quals(materialize_htid,
tbinfo->htpartcoltype,
Expand Down
1 change: 0 additions & 1 deletion tsl/src/continuous_aggs/common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
#include "ts_catalog/catalog.h"
#include "ts_catalog/continuous_agg.h"

#define CONTINUOUS_AGG_MAX_JOIN_RELATIONS 2
#define DEFAULT_MATPARTCOLUMN_NAME "time_partition_col"
#define CAGG_INVALIDATION_THRESHOLD_NAME "invalidation threshold watermark"

Expand Down
11 changes: 2 additions & 9 deletions tsl/src/continuous_aggs/finalize.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,7 @@ finalizequery_get_select_query(FinalizeQueryInfo *inp, List *matcollist,
* which contains the information of the materialised hypertable
* that is created for this cagg.
*/
if (list_length(inp->final_userquery->jointree->fromlist) >=
CONTINUOUS_AGG_MAX_JOIN_RELATIONS ||
if (list_length(inp->final_userquery->jointree->fromlist) >= 1 ||
!IsA(linitial(inp->final_userquery->jointree->fromlist), RangeTblRef))
{
rte = makeNode(RangeTblEntry);
Expand Down Expand Up @@ -259,8 +258,7 @@ finalizequery_get_select_query(FinalizeQueryInfo *inp, List *matcollist,
}
}

if (list_length(inp->final_userquery->jointree->fromlist) >=
CONTINUOUS_AGG_MAX_JOIN_RELATIONS ||
if (list_length(inp->final_userquery->jointree->fromlist) >= 1 ||
!IsA(linitial(inp->final_userquery->jointree->fromlist), RangeTblRef))
{
RangeTblRef *rtr;
Expand Down Expand Up @@ -290,15 +288,10 @@ finalizequery_get_select_query(FinalizeQueryInfo *inp, List *matcollist,
* we can not copy the fromlist from inp->final_userquery as
* it has two tables in this case.
*/
Assert(list_length(inp->final_userquery->jointree->fromlist) <=
CONTINUOUS_AGG_MAX_JOIN_RELATIONS);

final_selquery->jointree = fromexpr;
final_selquery->targetList = inp->final_seltlist;
final_selquery->sortClause = inp->final_userquery->sortClause;

/* Already finalized query no need to copy group by or having clause. */

return final_selquery;
}

Expand Down
47 changes: 47 additions & 0 deletions tsl/test/expected/cagg_joins.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1165,6 +1165,53 @@ JOIN location ON location.name = devices.location
GROUP BY bucket, devices.name, location.name;
NOTICE: refreshing continuous aggregate "conditions_by_day"
HINT: Use WITH NO DATA if you do not want to refresh the continuous aggregate on creation.
SELECT * FROM conditions_by_day ORDER BY bucket, device, location;
bucket | avg | device | location
------------------------------+---------------------+----------+-----------
Sun Jun 13 17:00:00 2021 PDT | 26.0000000000000000 | thermo_1 | Moscow
Mon Jun 14 17:00:00 2021 PDT | 22.0000000000000000 | thermo_2 | Berlin
Tue Jun 15 17:00:00 2021 PDT | 24.0000000000000000 | thermo_3 | London
Wed Jun 16 17:00:00 2021 PDT | 24.0000000000000000 | thermo_4 | Stockholm
Thu Jun 17 17:00:00 2021 PDT | 27.0000000000000000 | thermo_4 | Stockholm
Fri Jun 18 17:00:00 2021 PDT | 28.0000000000000000 | thermo_4 | Stockholm
Sat Jun 19 17:00:00 2021 PDT | 30.0000000000000000 | thermo_1 | Moscow
Sun Jun 20 17:00:00 2021 PDT | 31.0000000000000000 | thermo_1 | Moscow
Mon Jun 21 17:00:00 2021 PDT | 34.0000000000000000 | thermo_1 | Moscow
Tue Jun 22 17:00:00 2021 PDT | 34.0000000000000000 | thermo_2 | Berlin
Wed Jun 23 17:00:00 2021 PDT | 34.0000000000000000 | thermo_2 | Berlin
Thu Jun 24 17:00:00 2021 PDT | 32.0000000000000000 | thermo_3 | London
Fri Jun 25 17:00:00 2021 PDT | 32.0000000000000000 | thermo_3 | London
Sat Jun 26 17:00:00 2021 PDT | 31.0000000000000000 | thermo_3 | London
Tue Jun 29 17:00:00 2021 PDT | 28.0000000000000000 | thermo_3 | London
Wed Jun 30 17:00:00 2021 PDT | 28.0000000000000000 | thermo_3 | London
(16 rows)

ALTER MATERIALIZED VIEW conditions_by_day SET (timescaledb.materialized_only = FALSE);
-- Insert one more row on conditions and check the count
INSERT INTO conditions (day, city, temperature, device_id) VALUES
('2024-07-01', 'Moscow', 28, 3);
SELECT * FROM conditions_by_day ORDER BY bucket, device, location;
bucket | avg | device | location
------------------------------+---------------------+----------+-----------
Sun Jun 13 17:00:00 2021 PDT | 26.0000000000000000 | thermo_1 | Moscow
Mon Jun 14 17:00:00 2021 PDT | 22.0000000000000000 | thermo_2 | Berlin
Tue Jun 15 17:00:00 2021 PDT | 24.0000000000000000 | thermo_3 | London
Wed Jun 16 17:00:00 2021 PDT | 24.0000000000000000 | thermo_4 | Stockholm
Thu Jun 17 17:00:00 2021 PDT | 27.0000000000000000 | thermo_4 | Stockholm
Fri Jun 18 17:00:00 2021 PDT | 28.0000000000000000 | thermo_4 | Stockholm
Sat Jun 19 17:00:00 2021 PDT | 30.0000000000000000 | thermo_1 | Moscow
Sun Jun 20 17:00:00 2021 PDT | 31.0000000000000000 | thermo_1 | Moscow
Mon Jun 21 17:00:00 2021 PDT | 34.0000000000000000 | thermo_1 | Moscow
Tue Jun 22 17:00:00 2021 PDT | 34.0000000000000000 | thermo_2 | Berlin
Wed Jun 23 17:00:00 2021 PDT | 34.0000000000000000 | thermo_2 | Berlin
Thu Jun 24 17:00:00 2021 PDT | 32.0000000000000000 | thermo_3 | London
Fri Jun 25 17:00:00 2021 PDT | 32.0000000000000000 | thermo_3 | London
Sat Jun 26 17:00:00 2021 PDT | 31.0000000000000000 | thermo_3 | London
Tue Jun 29 17:00:00 2021 PDT | 28.0000000000000000 | thermo_3 | London
Wed Jun 30 17:00:00 2021 PDT | 28.0000000000000000 | thermo_3 | London
Sun Jun 30 17:00:00 2024 PDT | 28.0000000000000000 | thermo_3 | London
(17 rows)

-- JOIN with a foreign table
\c :TEST_DBNAME :ROLE_SUPERUSER
SELECT current_setting('port') AS "PGPORT", current_database() AS "PGDATABASE" \gset
Expand Down
10 changes: 10 additions & 0 deletions tsl/test/sql/cagg_joins.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,16 @@ JOIN devices ON conditions.device_id = devices.device_id
JOIN location ON location.name = devices.location
GROUP BY bucket, devices.name, location.name;

SELECT * FROM conditions_by_day ORDER BY bucket, device, location;

ALTER MATERIALIZED VIEW conditions_by_day SET (timescaledb.materialized_only = FALSE);

-- Insert one more row on conditions and check the count
INSERT INTO conditions (day, city, temperature, device_id) VALUES
('2024-07-01', 'Moscow', 28, 3);

SELECT * FROM conditions_by_day ORDER BY bucket, device, location;

-- JOIN with a foreign table
\c :TEST_DBNAME :ROLE_SUPERUSER
SELECT current_setting('port') AS "PGPORT", current_database() AS "PGDATABASE" \gset
Expand Down

0 comments on commit 971c6ed

Please sign in to comment.