Skip to content

Commit bb7bd63

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)
1 parent f8e7692 commit bb7bd63

14 files changed

Lines changed: 598 additions & 4 deletions

‎src/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ set(IMPORTED_SOURCES
135135
import/createplan.c
136136
import/heapswap.c
137137
import/list.c
138+
import/plancat.c
138139
import/planner.c
139140
import/setrefs.c
140141
import/ts_explain.c

‎src/compat/compat.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,8 @@ initReadOnlyStringInfo(StringInfo str, char *data, int len)
727727
#define COMPARE_LT BTLessStrategyNumber
728728
#define COMPARE_GT BTGreaterStrategyNumber
729729
#define pk_cmptype pk_strategy
730+
#define get_opfamily_member_for_cmptype(opfamily, lefttype, righttype, cmptype) \
731+
get_opfamily_member(opfamily, lefttype, righttype, cmptype)
730732
#endif
731733

732734
/* PG18 adds is_merge_delete param to ExecBR{Delete|Update}Triggers function.

‎src/import/plancat.c‎

Lines changed: 337 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,337 @@
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 <access/amapi.h>
15+
#include <access/genam.h>
16+
#include <access/htup_details.h>
17+
#include <access/table.h>
18+
#include <access/tableam.h>
19+
#include <access/transam.h>
20+
#include <access/xact.h>
21+
#include <catalog/heap.h>
22+
#include <catalog/index.h>
23+
#include <catalog/pg_am.h>
24+
#include <catalog/pg_attribute.h>
25+
#include <nodes/makefuncs.h>
26+
#include <nodes/nodeFuncs.h>
27+
#include <nodes/pathnodes.h>
28+
#include <optimizer/optimizer.h>
29+
#include <optimizer/plancat.h>
30+
#include <parser/parsetree.h>
31+
#include <rewrite/rewriteManip.h>
32+
#include <storage/bufmgr.h>
33+
#include <utils/lsyscache.h>
34+
#include <utils/rel.h>
35+
#include <utils/snapmgr.h>
36+
#include <utils/syscache.h>
37+
38+
#include "compat/compat.h"
39+
#include "plancat.h"
40+
41+
/*
42+
* Copied verbatim from build_index_tlist() in
43+
* src/backend/optimizer/util/plancat.c.
44+
*/
45+
static List *
46+
build_index_tlist(PlannerInfo *root, IndexOptInfo *index, Relation heapRelation)
47+
{
48+
List *tlist = NIL;
49+
Index varno = index->rel->relid;
50+
ListCell *indexpr_item;
51+
int i;
52+
53+
indexpr_item = list_head(index->indexprs);
54+
for (i = 0; i < index->ncolumns; i++)
55+
{
56+
int indexkey = index->indexkeys[i];
57+
Expr *indexvar;
58+
59+
if (indexkey != 0)
60+
{
61+
/* simple column */
62+
const FormData_pg_attribute *att_tup;
63+
64+
if (indexkey < 0)
65+
att_tup = SystemAttributeDefinition(indexkey);
66+
else
67+
att_tup = TupleDescAttr(heapRelation->rd_att, indexkey - 1);
68+
69+
indexvar = (Expr *) makeVar(varno,
70+
indexkey,
71+
att_tup->atttypid,
72+
att_tup->atttypmod,
73+
att_tup->attcollation,
74+
0);
75+
}
76+
else
77+
{
78+
/* expression column */
79+
if (indexpr_item == NULL)
80+
elog(ERROR, "wrong number of index expressions");
81+
indexvar = (Expr *) lfirst(indexpr_item);
82+
indexpr_item = lnext(index->indexprs, indexpr_item);
83+
}
84+
85+
tlist = lappend(tlist, makeTargetEntry(indexvar, i + 1, NULL, false));
86+
}
87+
if (indexpr_item != NULL)
88+
elog(ERROR, "wrong number of index expressions");
89+
90+
return tlist;
91+
}
92+
93+
/*
94+
* Copied and trimmed down from get_relation_info() in
95+
* src/backend/optimizer/util/plancat.c. Postgres itself skips building
96+
* rel->indexlist for a relation whose RTE it considers an inheritance parent.
97+
*
98+
* We call this to build the indexlist for hypertables ourselves because
99+
* some planner optimizations that run before our own hypertable expansion
100+
* (e.g. useless-join and self-join elimination) need a populated indexlist to
101+
* prove the relation unique.
102+
*
103+
* Unlike get_relation_info(), this does not populate rel->tuples/pages (that
104+
* stays 0 for a hypertable parent, to be filled in later from its chunks),
105+
* so per-index tuple estimates copied from rel->tuples are unreliable here.
106+
*/
107+
void
108+
ts_build_indexlist(PlannerInfo *root, RelOptInfo *rel)
109+
{
110+
Index varno = rel->relid;
111+
RangeTblEntry *rte = planner_rt_fetch(varno, root);
112+
Relation relation;
113+
List *indexinfos = NIL;
114+
List *indexoidlist;
115+
LOCKMODE lmode;
116+
ListCell *l;
117+
118+
relation = table_open(rte->relid, NoLock);
119+
120+
if (!relation->rd_rel->relhasindex)
121+
{
122+
table_close(relation, NoLock);
123+
return;
124+
}
125+
126+
indexoidlist = RelationGetIndexList(relation);
127+
lmode = rte->rellockmode;
128+
129+
foreach (l, indexoidlist)
130+
{
131+
Oid indexoid = lfirst_oid(l);
132+
Relation indexRelation;
133+
Form_pg_index index;
134+
IndexAmRoutine *amroutine = NULL;
135+
IndexOptInfo *info;
136+
int ncolumns, nkeycolumns;
137+
int i;
138+
139+
indexRelation = index_open(indexoid, lmode);
140+
index = indexRelation->rd_index;
141+
142+
if (!index->indisvalid)
143+
{
144+
index_close(indexRelation, NoLock);
145+
continue;
146+
}
147+
148+
if (index->indcheckxmin &&
149+
!TransactionIdPrecedes(HeapTupleHeaderGetXmin(indexRelation->rd_indextuple->t_data),
150+
TransactionXmin))
151+
{
152+
root->glob->transientPlan = true;
153+
index_close(indexRelation, NoLock);
154+
continue;
155+
}
156+
157+
info = makeNode(IndexOptInfo);
158+
159+
info->indexoid = index->indexrelid;
160+
info->reltablespace = RelationGetForm(indexRelation)->reltablespace;
161+
info->rel = rel;
162+
info->ncolumns = ncolumns = index->indnatts;
163+
info->nkeycolumns = nkeycolumns = index->indnkeyatts;
164+
165+
info->indexkeys = (int *) palloc(sizeof(int) * ncolumns);
166+
info->indexcollations = (Oid *) palloc(sizeof(Oid) * nkeycolumns);
167+
info->opfamily = (Oid *) palloc(sizeof(Oid) * nkeycolumns);
168+
info->opcintype = (Oid *) palloc(sizeof(Oid) * nkeycolumns);
169+
info->canreturn = (bool *) palloc(sizeof(bool) * ncolumns);
170+
171+
for (i = 0; i < ncolumns; i++)
172+
{
173+
info->indexkeys[i] = index->indkey.values[i];
174+
info->canreturn[i] = index_can_return(indexRelation, i + 1);
175+
}
176+
177+
for (i = 0; i < nkeycolumns; i++)
178+
{
179+
info->opfamily[i] = indexRelation->rd_opfamily[i];
180+
info->opcintype[i] = indexRelation->rd_opcintype[i];
181+
info->indexcollations[i] = indexRelation->rd_indcollation[i];
182+
}
183+
184+
info->relam = indexRelation->rd_rel->relam;
185+
186+
if (indexRelation->rd_rel->relkind != RELKIND_PARTITIONED_INDEX)
187+
{
188+
amroutine = indexRelation->rd_indam;
189+
info->amcanorderbyop = amroutine->amcanorderbyop;
190+
info->amoptionalkey = amroutine->amoptionalkey;
191+
info->amsearcharray = amroutine->amsearcharray;
192+
info->amsearchnulls = amroutine->amsearchnulls;
193+
info->amcanparallel = amroutine->amcanparallel;
194+
info->amhasgettuple = (amroutine->amgettuple != NULL);
195+
info->amhasgetbitmap =
196+
amroutine->amgetbitmap != NULL && relation->rd_tableam->scan_bitmap_next_tuple != NULL;
197+
info->amcanmarkpos =
198+
(amroutine->ammarkpos != NULL && amroutine->amrestrpos != NULL);
199+
info->amcostestimate = amroutine->amcostestimate;
200+
Assert(info->amcostestimate != NULL);
201+
202+
info->opclassoptions = RelationGetIndexAttOptions(indexRelation, true);
203+
204+
if (info->relam == BTREE_AM_OID)
205+
{
206+
Assert(amroutine->amcanorder);
207+
208+
info->sortopfamily = info->opfamily;
209+
info->reverse_sort = (bool *) palloc(sizeof(bool) * nkeycolumns);
210+
info->nulls_first = (bool *) palloc(sizeof(bool) * nkeycolumns);
211+
212+
for (i = 0; i < nkeycolumns; i++)
213+
{
214+
int16 opt = indexRelation->rd_indoption[i];
215+
216+
info->reverse_sort[i] = (opt & INDOPTION_DESC) != 0;
217+
info->nulls_first[i] = (opt & INDOPTION_NULLS_FIRST) != 0;
218+
}
219+
}
220+
else if (amroutine->amcanorder)
221+
{
222+
info->sortopfamily = (Oid *) palloc(sizeof(Oid) * nkeycolumns);
223+
info->reverse_sort = (bool *) palloc(sizeof(bool) * nkeycolumns);
224+
info->nulls_first = (bool *) palloc(sizeof(bool) * nkeycolumns);
225+
226+
for (i = 0; i < nkeycolumns; i++)
227+
{
228+
int16 opt = indexRelation->rd_indoption[i];
229+
Oid ltopr;
230+
Oid opfamily;
231+
Oid opcintype;
232+
CompareType cmptype;
233+
234+
info->reverse_sort[i] = (opt & INDOPTION_DESC) != 0;
235+
info->nulls_first[i] = (opt & INDOPTION_NULLS_FIRST) != 0;
236+
237+
ltopr = get_opfamily_member_for_cmptype(info->opfamily[i],
238+
info->opcintype[i],
239+
info->opcintype[i],
240+
COMPARE_LT);
241+
if (OidIsValid(ltopr) &&
242+
get_ordering_op_properties(ltopr, &opfamily, &opcintype, &cmptype) &&
243+
opcintype == info->opcintype[i] && cmptype == COMPARE_LT)
244+
{
245+
info->sortopfamily[i] = opfamily;
246+
}
247+
else
248+
{
249+
info->sortopfamily = NULL;
250+
info->reverse_sort = NULL;
251+
info->nulls_first = NULL;
252+
break;
253+
}
254+
}
255+
}
256+
else
257+
{
258+
info->sortopfamily = NULL;
259+
info->reverse_sort = NULL;
260+
info->nulls_first = NULL;
261+
}
262+
}
263+
else
264+
{
265+
info->amcanorderbyop = false;
266+
info->amoptionalkey = false;
267+
info->amsearcharray = false;
268+
info->amsearchnulls = false;
269+
info->amcanparallel = false;
270+
info->amhasgettuple = false;
271+
info->amhasgetbitmap = false;
272+
info->amcanmarkpos = false;
273+
info->amcostestimate = NULL;
274+
275+
info->sortopfamily = NULL;
276+
info->reverse_sort = NULL;
277+
info->nulls_first = NULL;
278+
}
279+
280+
info->indexprs = RelationGetIndexExpressions(indexRelation);
281+
info->indpred = RelationGetIndexPredicate(indexRelation);
282+
if (info->indexprs && varno != 1)
283+
ChangeVarNodes((Node *) info->indexprs, 1, varno, 0);
284+
if (info->indpred && varno != 1)
285+
ChangeVarNodes((Node *) info->indpred, 1, varno, 0);
286+
287+
info->indextlist = build_index_tlist(root, info, relation);
288+
289+
info->indrestrictinfo = NIL;
290+
info->predOK = false;
291+
info->unique = index->indisunique;
292+
#if PG18_GE
293+
info->nullsnotdistinct = index->indnullsnotdistinct;
294+
#endif
295+
info->immediate = index->indimmediate;
296+
info->hypothetical = false;
297+
298+
if (indexRelation->rd_rel->relkind != RELKIND_PARTITIONED_INDEX)
299+
{
300+
if (info->indpred == NIL)
301+
{
302+
info->pages = RelationGetNumberOfBlocks(indexRelation);
303+
info->tuples = rel->tuples;
304+
}
305+
else
306+
{
307+
double allvisfrac; /* dummy */
308+
309+
estimate_rel_size(indexRelation, NULL, &info->pages, &info->tuples, &allvisfrac);
310+
if (info->tuples > rel->tuples)
311+
info->tuples = rel->tuples;
312+
}
313+
314+
#if PG18_GE
315+
if (amroutine->amgettreeheight)
316+
info->tree_height = amroutine->amgettreeheight(indexRelation);
317+
else
318+
#endif
319+
info->tree_height = -1;
320+
}
321+
else
322+
{
323+
info->pages = 0;
324+
info->tuples = 0.0;
325+
info->tree_height = -1;
326+
}
327+
328+
index_close(indexRelation, NoLock);
329+
330+
indexinfos = lcons(info, indexinfos);
331+
}
332+
333+
list_free(indexoidlist);
334+
table_close(relation, NoLock);
335+
336+
rel->indexlist = indexinfos;
337+
}

‎src/import/plancat.h‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
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/pathnodes.h>
17+
18+
extern void ts_build_indexlist(PlannerInfo *root, RelOptInfo *rel);

‎src/planner/planner.c‎

Lines changed: 12 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/plancat.h"
5354
#include "license_guc.h"
5455
#include "nodes/chunk_append/chunk_append.h"
5556
#include "nodes/constraint_aware_append/constraint_aware_append.h"
@@ -1616,6 +1617,17 @@ timescaledb_get_relation_info(PlannerInfo *root, RelOptInfo *rel, bool inhparent
16161617
*/
16171618
Assert(!IS_DUMMY_REL(rel));
16181619

1620+
/*
1621+
* Postgres skips building rel->indexlist for a relation it
1622+
* considers an inheritance parent. But useless-join and self-join
1623+
* elimination need a populated indexlist to prove this relation is
1624+
* unique.
1625+
*/
1626+
if (ts_guc_enable_optimizations && inhparent)
1627+
{
1628+
ts_build_indexlist(root, rel);
1629+
}
1630+
16191631
/*
16201632
* Mark hypertable RTEs we'd like to expand ourselves. We do this
16211633
* for hypertables participating SELECT, UPDATE and DELETE,

0 commit comments

Comments
 (0)