Skip to content

Commit c300d2e

Browse files
committed
Fix hypertables not getting removed from useless outer joins
If a hypertable only appears on the nullable side of a LEFT/RIGHT/FULL join, and none of its columns are used elsewhere in the query, Postgres can normally drop the join entirely. This stopped working after 390d900 (Run our hypertable expansion only from the get_relation_info_hook, #9714), which removed marking a hypertable early enough for Postgres to still see it as a plain table.
1 parent f8e7692 commit c300d2e

9 files changed

Lines changed: 283 additions & 0 deletions

src/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ set(IMPORTED_SOURCES
136136
import/heapswap.c
137137
import/list.c
138138
import/planner.c
139+
import/prepjointree.c
139140
import/setrefs.c
140141
import/ts_explain.c
141142
import/ts_inherit.c)

src/import/prepjointree.c

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/*
2+
* This file and its contents are licensed under the Apache License 2.0.
3+
* Please see the included NOTICE for copyright information and
4+
* LICENSE-APACHE for a copy of the license.
5+
*/
6+
7+
/*
8+
* This file contains source code that was copied and/or modified from
9+
* the PostgreSQL database, which is licensed under the open-source
10+
* PostgreSQL License. Please see the NOTICE at the top level
11+
* directory for a copy of the PostgreSQL License.
12+
*/
13+
#include <postgres.h>
14+
#include <nodes/bitmapset.h>
15+
#include <nodes/nodes.h>
16+
#include <nodes/parsenodes.h>
17+
#include <nodes/pathnodes.h>
18+
#include <nodes/primnodes.h>
19+
20+
#include "prepjointree.h"
21+
22+
/*
23+
* Copied verbatim from src/backend/optimizer/prep/prepjointree.c.
24+
*/
25+
typedef struct nullingrel_info
26+
{
27+
/*
28+
* For each leaf RTE, nullingrels[rti] is the set of relids of outer joins
29+
* that potentially null that RTE.
30+
*/
31+
Relids *nullingrels;
32+
/* Length of range table (maximum index in nullingrels[]) */
33+
int rtlength; /* used only for assertion checks */
34+
} nullingrel_info;
35+
36+
static void get_nullingrels_recurse(Node *jtnode, Relids upper_nullingrels,
37+
nullingrel_info *info);
38+
39+
/*
40+
* get_nullingrels: collect info about which outer joins null which relations
41+
*
42+
* The result struct contains, for each leaf relation used in the query,
43+
* the set of relids of outer joins that potentially null that rel.
44+
*/
45+
static nullingrel_info *
46+
get_nullingrels(Query *parse)
47+
{
48+
nullingrel_info *result = palloc_object(nullingrel_info);
49+
50+
result->rtlength = list_length(parse->rtable);
51+
result->nullingrels = palloc0_array(Relids, result->rtlength + 1);
52+
get_nullingrels_recurse((Node *) parse->jointree, NULL, result);
53+
return result;
54+
}
55+
56+
/*
57+
* Recursive guts of get_nullingrels().
58+
*
59+
* Note: at any recursion level, the passed-down upper_nullingrels must be
60+
* treated as a constant, but it can be stored directly into *info
61+
* if we're at leaf level. Upper recursion levels do not free their mutated
62+
* copies of the nullingrels, because those are probably referenced by
63+
* at least one leaf rel.
64+
*/
65+
static void
66+
get_nullingrels_recurse(Node *jtnode, Relids upper_nullingrels, nullingrel_info *info)
67+
{
68+
if (jtnode == NULL)
69+
return;
70+
if (IsA(jtnode, RangeTblRef))
71+
{
72+
int varno = ((RangeTblRef *) jtnode)->rtindex;
73+
74+
Assert(varno > 0 && varno <= info->rtlength);
75+
info->nullingrels[varno] = upper_nullingrels;
76+
}
77+
else if (IsA(jtnode, FromExpr))
78+
{
79+
FromExpr *f = (FromExpr *) jtnode;
80+
ListCell *l;
81+
82+
foreach (l, f->fromlist)
83+
{
84+
get_nullingrels_recurse(lfirst(l), upper_nullingrels, info);
85+
}
86+
}
87+
else if (IsA(jtnode, JoinExpr))
88+
{
89+
JoinExpr *j = (JoinExpr *) jtnode;
90+
Relids local_nullingrels;
91+
92+
switch (j->jointype)
93+
{
94+
case JOIN_INNER:
95+
get_nullingrels_recurse(j->larg, upper_nullingrels, info);
96+
get_nullingrels_recurse(j->rarg, upper_nullingrels, info);
97+
break;
98+
case JOIN_LEFT:
99+
case JOIN_SEMI:
100+
case JOIN_ANTI:
101+
local_nullingrels = bms_add_member(bms_copy(upper_nullingrels), j->rtindex);
102+
get_nullingrels_recurse(j->larg, upper_nullingrels, info);
103+
get_nullingrels_recurse(j->rarg, local_nullingrels, info);
104+
break;
105+
case JOIN_FULL:
106+
local_nullingrels = bms_add_member(bms_copy(upper_nullingrels), j->rtindex);
107+
get_nullingrels_recurse(j->larg, local_nullingrels, info);
108+
get_nullingrels_recurse(j->rarg, local_nullingrels, info);
109+
break;
110+
case JOIN_RIGHT:
111+
local_nullingrels = bms_add_member(bms_copy(upper_nullingrels), j->rtindex);
112+
get_nullingrels_recurse(j->larg, local_nullingrels, info);
113+
get_nullingrels_recurse(j->rarg, upper_nullingrels, info);
114+
break;
115+
default:
116+
elog(ERROR, "unrecognized join type: %d", (int) j->jointype);
117+
break;
118+
}
119+
}
120+
else
121+
elog(ERROR, "unrecognized node type: %d", (int) nodeTag(jtnode));
122+
}
123+
124+
/*
125+
* Return the set of rtindexes of relations that can be NULL-extended by some
126+
* outer join in the query's jointree.
127+
*/
128+
Bitmapset *
129+
ts_get_nullable_rtis(Query *parse)
130+
{
131+
nullingrel_info *info = get_nullingrels(parse);
132+
Bitmapset *nullable_rtis = NULL;
133+
134+
for (int rti = 1; rti <= info->rtlength; rti++)
135+
{
136+
if (!bms_is_empty(info->nullingrels[rti]))
137+
nullable_rtis = bms_add_member(nullable_rtis, rti);
138+
}
139+
140+
return nullable_rtis;
141+
}

src/import/prepjointree.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/*
2+
* This file and its contents are licensed under the Apache License 2.0.
3+
* Please see the included NOTICE for copyright information and
4+
* LICENSE-APACHE for a copy of the license.
5+
*/
6+
7+
/*
8+
* This file contains source code that was copied and/or modified from
9+
* the PostgreSQL database, which is licensed under the open-source
10+
* PostgreSQL License. Please see the NOTICE at the top level
11+
* directory for a copy of the PostgreSQL License.
12+
*/
13+
#pragma once
14+
15+
#include <postgres.h>
16+
#include <nodes/bitmapset.h>
17+
#include <nodes/parsenodes.h>
18+
19+
extern Bitmapset *ts_get_nullable_rtis(Query *parse);

src/planner/planner.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
#include "hypertable.h"
5151
#include "hypertable_cache.h"
5252
#include "import/allpaths.h"
53+
#include "import/prepjointree.h"
5354
#include "license_guc.h"
5455
#include "nodes/chunk_append/chunk_append.h"
5556
#include "nodes/constraint_aware_append/constraint_aware_append.h"
@@ -444,6 +445,7 @@ preprocess_query(Node *node, PreprocessQueryContext *context)
444445
ListCell *lc;
445446
Index rti = 1;
446447
bool ret;
448+
Bitmapset *nullable_rtis = ts_get_nullable_rtis(query);
447449

448450
if (ts_guc_enable_foreign_key_propagation)
449451
{
@@ -490,6 +492,21 @@ preprocess_query(Node *node, PreprocessQueryContext *context)
490492
rte_mark_for_expansion(rte);
491493
}
492494
}
495+
else if (bms_is_member(rti, nullable_rtis))
496+
{
497+
/*
498+
* Mark hypertable RTEs on the nullable side of an outer
499+
* join here too: get_relation_info_hook runs after join
500+
* removal, so a hypertable only visible via view inlining
501+
* would still look like an inheritance parent when join
502+
* removal runs, blocking useless-join elimination.
503+
*/
504+
if (ts_guc_enable_optimizations && ts_guc_enable_constraint_exclusion &&
505+
rte->inh && (Index) query->resultRelation != rti)
506+
{
507+
rte_mark_for_expansion(rte);
508+
}
509+
}
493510
break;
494511
default:
495512
break;

test/expected/plan_expand_hypertable-16.out

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3671,4 +3671,26 @@ EXPLAIN (buffers off, costs off, timing off, summary off, analyze) SELECT * FROM
36713671
-> Index Scan using _hyper_21_203_chunk_metrics_space_time_idx on _hyper_21_203_chunk (actual rows=1.00 loops=1)
36723672
Index Cond: ("time" >= 'Sun Jan 02 00:00:00 2000 UTC'::timestamp with time zone)
36733673

3674+
CREATE TABLE join_removal_main(time timestamptz NOT NULL, device int, val float);
3675+
SELECT create_hypertable('join_removal_main', 'time');
3676+
create_hypertable
3677+
---------------------------------
3678+
(22,public,join_removal_main,t)
3679+
3680+
CREATE TABLE join_removal_aux(time timestamptz NOT NULL, device int NOT NULL, extra float);
3681+
SELECT create_hypertable('join_removal_aux', 'time');
3682+
create_hypertable
3683+
--------------------------------
3684+
(23,public,join_removal_aux,t)
3685+
3686+
CREATE UNIQUE INDEX join_removal_aux_uq ON join_removal_aux (device, time);
3687+
INSERT INTO join_removal_main VALUES ('2021-01-01', 1, 1.0), ('2021-01-02', 2, 2.0);
3688+
INSERT INTO join_removal_aux VALUES ('2021-01-01', 1, 10.0), ('2021-01-02', 2, 20.0);
3689+
ANALYZE join_removal_main, join_removal_aux;
3690+
EXPLAIN (buffers off, costs off, timing off, summary off, analyze) SELECT m.time, m.device, m.val
3691+
FROM join_removal_main m
3692+
LEFT JOIN join_removal_aux a ON a.device = m.device AND a.time = m.time;
3693+
--- QUERY PLAN ---
3694+
Seq Scan on _hyper_22_204_chunk m (actual rows=2.00 loops=1)
3695+
36743696
--TEST END--

test/expected/plan_expand_hypertable-17.out

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3671,4 +3671,26 @@ EXPLAIN (buffers off, costs off, timing off, summary off, analyze) SELECT * FROM
36713671
-> Index Scan using _hyper_21_203_chunk_metrics_space_time_idx on _hyper_21_203_chunk (actual rows=1.00 loops=1)
36723672
Index Cond: ("time" >= 'Sun Jan 02 00:00:00 2000 UTC'::timestamp with time zone)
36733673

3674+
CREATE TABLE join_removal_main(time timestamptz NOT NULL, device int, val float);
3675+
SELECT create_hypertable('join_removal_main', 'time');
3676+
create_hypertable
3677+
---------------------------------
3678+
(22,public,join_removal_main,t)
3679+
3680+
CREATE TABLE join_removal_aux(time timestamptz NOT NULL, device int NOT NULL, extra float);
3681+
SELECT create_hypertable('join_removal_aux', 'time');
3682+
create_hypertable
3683+
--------------------------------
3684+
(23,public,join_removal_aux,t)
3685+
3686+
CREATE UNIQUE INDEX join_removal_aux_uq ON join_removal_aux (device, time);
3687+
INSERT INTO join_removal_main VALUES ('2021-01-01', 1, 1.0), ('2021-01-02', 2, 2.0);
3688+
INSERT INTO join_removal_aux VALUES ('2021-01-01', 1, 10.0), ('2021-01-02', 2, 20.0);
3689+
ANALYZE join_removal_main, join_removal_aux;
3690+
EXPLAIN (buffers off, costs off, timing off, summary off, analyze) SELECT m.time, m.device, m.val
3691+
FROM join_removal_main m
3692+
LEFT JOIN join_removal_aux a ON a.device = m.device AND a.time = m.time;
3693+
--- QUERY PLAN ---
3694+
Seq Scan on _hyper_22_204_chunk m (actual rows=2.00 loops=1)
3695+
36743696
--TEST END--

test/expected/plan_expand_hypertable-18.out

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3671,4 +3671,26 @@ EXPLAIN (buffers off, costs off, timing off, summary off, analyze) SELECT * FROM
36713671
-> Index Scan using _hyper_21_203_chunk_metrics_space_time_idx on _hyper_21_203_chunk (actual rows=1.00 loops=1)
36723672
Index Cond: ("time" >= 'Sun Jan 02 00:00:00 2000 UTC'::timestamp with time zone)
36733673

3674+
CREATE TABLE join_removal_main(time timestamptz NOT NULL, device int, val float);
3675+
SELECT create_hypertable('join_removal_main', 'time');
3676+
create_hypertable
3677+
---------------------------------
3678+
(22,public,join_removal_main,t)
3679+
3680+
CREATE TABLE join_removal_aux(time timestamptz NOT NULL, device int NOT NULL, extra float);
3681+
SELECT create_hypertable('join_removal_aux', 'time');
3682+
create_hypertable
3683+
--------------------------------
3684+
(23,public,join_removal_aux,t)
3685+
3686+
CREATE UNIQUE INDEX join_removal_aux_uq ON join_removal_aux (device, time);
3687+
INSERT INTO join_removal_main VALUES ('2021-01-01', 1, 1.0), ('2021-01-02', 2, 2.0);
3688+
INSERT INTO join_removal_aux VALUES ('2021-01-01', 1, 10.0), ('2021-01-02', 2, 20.0);
3689+
ANALYZE join_removal_main, join_removal_aux;
3690+
EXPLAIN (buffers off, costs off, timing off, summary off, analyze) SELECT m.time, m.device, m.val
3691+
FROM join_removal_main m
3692+
LEFT JOIN join_removal_aux a ON a.device = m.device AND a.time = m.time;
3693+
--- QUERY PLAN ---
3694+
Seq Scan on _hyper_22_204_chunk m (actual rows=2.00 loops=1)
3695+
36743696
--TEST END--

test/expected/plan_expand_hypertable-19.out

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3670,4 +3670,26 @@ EXPLAIN (buffers off, costs off, timing off, summary off, analyze) SELECT * FROM
36703670
-> Index Scan using _hyper_21_203_chunk_metrics_space_time_idx on _hyper_21_203_chunk (actual rows=1.00 loops=1)
36713671
Index Cond: ("time" >= 'Sun Jan 02 00:00:00 2000 UTC'::timestamp with time zone)
36723672

3673+
CREATE TABLE join_removal_main(time timestamptz NOT NULL, device int, val float);
3674+
SELECT create_hypertable('join_removal_main', 'time');
3675+
create_hypertable
3676+
---------------------------------
3677+
(22,public,join_removal_main,t)
3678+
3679+
CREATE TABLE join_removal_aux(time timestamptz NOT NULL, device int NOT NULL, extra float);
3680+
SELECT create_hypertable('join_removal_aux', 'time');
3681+
create_hypertable
3682+
--------------------------------
3683+
(23,public,join_removal_aux,t)
3684+
3685+
CREATE UNIQUE INDEX join_removal_aux_uq ON join_removal_aux (device, time);
3686+
INSERT INTO join_removal_main VALUES ('2021-01-01', 1, 1.0), ('2021-01-02', 2, 2.0);
3687+
INSERT INTO join_removal_aux VALUES ('2021-01-01', 1, 10.0), ('2021-01-02', 2, 20.0);
3688+
ANALYZE join_removal_main, join_removal_aux;
3689+
EXPLAIN (buffers off, costs off, timing off, summary off, analyze) SELECT m.time, m.device, m.val
3690+
FROM join_removal_main m
3691+
LEFT JOIN join_removal_aux a ON a.device = m.device AND a.time = m.time;
3692+
--- QUERY PLAN ---
3693+
Seq Scan on _hyper_22_204_chunk m (actual rows=2.00 loops=1)
3694+
36733695
--TEST END--

test/sql/plan_expand_hypertable.sql.in

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,4 +172,21 @@ INSERT INTO metrics_space SELECT '2000-01-01'::timestamptz + format('%s day',i):
172172
-- no contraints removed due to space partitioning
173173
:PREFIX SELECT * FROM metrics_space WHERE time >= '2000-01-02';
174174

175+
-- test that a useless LEFT JOIN to a hypertable is eliminated when none of
176+
-- its columns are selected and the join clause matches a unique index
177+
CREATE TABLE join_removal_main(time timestamptz NOT NULL, device int, val float);
178+
SELECT create_hypertable('join_removal_main', 'time');
179+
180+
CREATE TABLE join_removal_aux(time timestamptz NOT NULL, device int NOT NULL, extra float);
181+
SELECT create_hypertable('join_removal_aux', 'time');
182+
CREATE UNIQUE INDEX join_removal_aux_uq ON join_removal_aux (device, time);
183+
184+
INSERT INTO join_removal_main VALUES ('2021-01-01', 1, 1.0), ('2021-01-02', 2, 2.0);
185+
INSERT INTO join_removal_aux VALUES ('2021-01-01', 1, 10.0), ('2021-01-02', 2, 20.0);
186+
ANALYZE join_removal_main, join_removal_aux;
187+
188+
:PREFIX SELECT m.time, m.device, m.val
189+
FROM join_removal_main m
190+
LEFT JOIN join_removal_aux a ON a.device = m.device AND a.time = m.time;
191+
175192
\qecho '--TEST END--'

0 commit comments

Comments
 (0)