Skip to content

Commit 6a0e5fe

Browse files
svenklemmsvenklemm
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 6a0e5fe

9 files changed

Lines changed: 592 additions & 308 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: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
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. Not
24+
* exported by Postgres for extensions to call.
25+
*/
26+
typedef struct nullingrel_info
27+
{
28+
/*
29+
* For each leaf RTE, nullingrels[rti] is the set of relids of outer joins
30+
* that potentially null that RTE.
31+
*/
32+
Relids *nullingrels;
33+
/* Length of range table (maximum index in nullingrels[]) */
34+
int rtlength; /* used only for assertion checks */
35+
} nullingrel_info;
36+
37+
static void get_nullingrels_recurse(Node *jtnode, Relids upper_nullingrels,
38+
nullingrel_info *info);
39+
40+
/*
41+
* get_nullingrels: collect info about which outer joins null which relations
42+
*
43+
* The result struct contains, for each leaf relation used in the query,
44+
* the set of relids of outer joins that potentially null that rel.
45+
*/
46+
static nullingrel_info *
47+
get_nullingrels(Query *parse)
48+
{
49+
nullingrel_info *result = palloc_object(nullingrel_info);
50+
51+
result->rtlength = list_length(parse->rtable);
52+
result->nullingrels = palloc0_array(Relids, result->rtlength + 1);
53+
get_nullingrels_recurse((Node *) parse->jointree, NULL, result);
54+
return result;
55+
}
56+
57+
/*
58+
* Recursive guts of get_nullingrels().
59+
*
60+
* Note: at any recursion level, the passed-down upper_nullingrels must be
61+
* treated as a constant, but it can be stored directly into *info
62+
* if we're at leaf level. Upper recursion levels do not free their mutated
63+
* copies of the nullingrels, because those are probably referenced by
64+
* at least one leaf rel.
65+
*/
66+
static void
67+
get_nullingrels_recurse(Node *jtnode, Relids upper_nullingrels, nullingrel_info *info)
68+
{
69+
if (jtnode == NULL)
70+
return;
71+
if (IsA(jtnode, RangeTblRef))
72+
{
73+
int varno = ((RangeTblRef *) jtnode)->rtindex;
74+
75+
Assert(varno > 0 && varno <= info->rtlength);
76+
info->nullingrels[varno] = upper_nullingrels;
77+
}
78+
else if (IsA(jtnode, FromExpr))
79+
{
80+
FromExpr *f = (FromExpr *) jtnode;
81+
ListCell *l;
82+
83+
foreach (l, f->fromlist)
84+
{
85+
get_nullingrels_recurse(lfirst(l), upper_nullingrels, info);
86+
}
87+
}
88+
else if (IsA(jtnode, JoinExpr))
89+
{
90+
JoinExpr *j = (JoinExpr *) jtnode;
91+
Relids local_nullingrels;
92+
93+
switch (j->jointype)
94+
{
95+
case JOIN_INNER:
96+
get_nullingrels_recurse(j->larg, upper_nullingrels, info);
97+
get_nullingrels_recurse(j->rarg, upper_nullingrels, info);
98+
break;
99+
case JOIN_LEFT:
100+
case JOIN_SEMI:
101+
case JOIN_ANTI:
102+
local_nullingrels = bms_add_member(bms_copy(upper_nullingrels), j->rtindex);
103+
get_nullingrels_recurse(j->larg, upper_nullingrels, info);
104+
get_nullingrels_recurse(j->rarg, local_nullingrels, info);
105+
break;
106+
case JOIN_FULL:
107+
local_nullingrels = bms_add_member(bms_copy(upper_nullingrels), j->rtindex);
108+
get_nullingrels_recurse(j->larg, local_nullingrels, info);
109+
get_nullingrels_recurse(j->rarg, local_nullingrels, info);
110+
break;
111+
case JOIN_RIGHT:
112+
local_nullingrels = bms_add_member(bms_copy(upper_nullingrels), j->rtindex);
113+
get_nullingrels_recurse(j->larg, local_nullingrels, info);
114+
get_nullingrels_recurse(j->rarg, upper_nullingrels, info);
115+
break;
116+
default:
117+
elog(ERROR, "unrecognized join type: %d", (int) j->jointype);
118+
break;
119+
}
120+
}
121+
else
122+
elog(ERROR, "unrecognized node type: %d", (int) nodeTag(jtnode));
123+
}
124+
125+
/*
126+
* Return the set of rtindexes of relations that can be NULL-extended by some
127+
* outer join in the query's jointree.
128+
*/
129+
Bitmapset *
130+
ts_get_nullable_rtis(Query *parse)
131+
{
132+
nullingrel_info *info = get_nullingrels(parse);
133+
Bitmapset *nullable_rtis = NULL;
134+
135+
for (int rti = 1; rti <= info->rtlength; rti++)
136+
{
137+
if (!bms_is_empty(info->nullingrels[rti]))
138+
nullable_rtis = bms_add_member(nullable_rtis, rti);
139+
}
140+
141+
return nullable_rtis;
142+
}

‎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 a real inheritance parent when
502+
* join 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;

0 commit comments

Comments
 (0)