Skip to content

Commit 390d900

Browse files
authored
Run our hypertable expansion only from the get_relation_info_hook (#9714)
We used to run it from preprocessing the query as well, but this complicates working with the DML target expansion code. Let the standard Postgres code run where possible, to reduce the divergence.
1 parent 45370c1 commit 390d900

9 files changed

Lines changed: 75 additions & 61 deletions

File tree

src/import/allpaths.c

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434

3535
#include "allpaths.h"
3636
#include "chunk.h"
37+
#include "compat/compat.h"
3738
#include "cross_module_fn.h"
3839
#include "planner/planner.h"
3940

@@ -224,6 +225,37 @@ ts_set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *parent_rel, Index pare
224225
parent_rel->consider_startup = true;
225226
}
226227

228+
#if PG16_LT
229+
/*
230+
* On PG15, create_append_path() calls get_appendrel_parampathinfo() instead
231+
* of the get_baserel_parampathinfo() like the later versions. The appendrel
232+
* parameterization info is not build for hypertable because of how we're
233+
* disabling the PG inheritance expansion, so here we have to compensate for
234+
* it. Fetch the parameterization info from the chunks here, following the
235+
* logic similar to add_paths_to_append_rel().
236+
*/
237+
if (parent_rel->reloptkind == RELOPT_BASEREL)
238+
{
239+
foreach (l, live_childrels)
240+
{
241+
RelOptInfo *child_rel = (RelOptInfo *) lfirst(l);
242+
ListCell *lcp;
243+
244+
foreach (lcp, child_rel->pathlist)
245+
{
246+
Path *child_path = (Path *) lfirst(lcp);
247+
248+
if (child_path->param_info == NULL)
249+
continue;
250+
251+
get_baserel_parampathinfo(root,
252+
parent_rel,
253+
child_path->param_info->ppi_req_outer);
254+
}
255+
}
256+
}
257+
#endif
258+
227259
/* Add paths to the append relation. */
228260
add_paths_to_append_rel(root, parent_rel, live_childrels);
229261
}

src/planner/planner.c

Lines changed: 17 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <access/tsmapi.h>
88
#include <access/xact.h>
99
#include <catalog/namespace.h>
10+
#include <catalog/pg_inherits.h>
1011
#include <commands/extension.h>
1112
#include <executor/nodeAgg.h>
1213
#include <miscadmin.h>
@@ -447,20 +448,10 @@ preprocess_query(Node *node, PreprocessQueryContext *context)
447448
if (ht)
448449
{
449450
/*
450-
* Mark hypertable RTEs we'd like to expand ourselves.
451-
* We always do this for SELECTs from hypertables.
452-
*
453-
* For DML, we also always expand the non-target relations.
454-
*
455-
* The hypertables that are not expanded by our custom code
456-
* here fall back to the standard Postgres inheritance
457-
* hierarchy expansion.
451+
* Hypertable expansion marking is done in the
452+
* get_relation_info_hook, which also handles
453+
* hypertables appearing after function or view inlining.
458454
*/
459-
if (ts_guc_enable_optimizations && ts_guc_enable_constraint_exclusion &&
460-
rte->inh && (Index) query->resultRelation != rti)
461-
{
462-
rte_mark_for_expansion(rte);
463-
}
464455

465456
if (TS_HYPERTABLE_HAS_COMPRESSION_TABLE(ht))
466457
{
@@ -1500,20 +1491,6 @@ timescaledb_set_rel_pathlist(PlannerInfo *root, RelOptInfo *rel, Index rti, Rang
15001491
break;
15011492

15021493
case TS_REL_HYPERTABLE:
1503-
/*
1504-
* Set the indexlist for a hypertable parent to NIL since we
1505-
* should not try to do any index scans on hypertable parents,
1506-
* similar to how it works for partitioned tables.
1507-
*
1508-
* This can happen when building a merge join path and computing
1509-
* cost for it. See get_actual_variable_range().
1510-
*
1511-
* This has to be after the hypertable is expanded, since the
1512-
* indexlist is used during hypertable expansion.
1513-
*/
1514-
1515-
rel->indexlist = NIL;
1516-
15171494
if (!rte->inh)
15181495
{
15191496
/*
@@ -1565,15 +1542,21 @@ timescaledb_get_relation_info_hook(PlannerInfo *root, Oid relation_objectid, boo
15651542
{
15661543
/*
15671544
* Mark hypertable RTEs we'd like to expand ourselves.
1568-
* Hypertables inside inlineable functions don't get marked during
1569-
* the query preprocessing step handled in preprocess_query().
1570-
* Therefore we do an extra try here.
1545+
* We always do this for SELECTs from hypertables.
1546+
*
1547+
* For DML, we also always expand the non-target relations.
1548+
*
1549+
* The hypertables that are not expanded by our custom code
1550+
* here fall back to the standard Postgres inheritance
1551+
* hierarchy expansion.
15711552
*
1572-
* For the explanation of the logic, see the comments in
1573-
* preprocess_query().
1553+
* `inhparent` goes to false in two cases: a hypertable without
1554+
* chunks or a SELECT FROM ONLY hypertable. We still want to run our
1555+
* hypertable expansion code for hypertables w/o chunks.
15741556
*/
1575-
if (ts_guc_enable_optimizations && ts_guc_enable_constraint_exclusion && inhparent &&
1576-
rte->ctename == NULL && rel->relid != (Index) query->resultRelation)
1557+
if (ts_guc_enable_optimizations && ts_guc_enable_constraint_exclusion &&
1558+
(inhparent || !has_subclass(rte->relid)) && rte->ctename == NULL &&
1559+
rel->relid != (Index) query->resultRelation)
15771560
{
15781561
rte_mark_for_expansion(rte);
15791562
}

test/expected/append-16.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1746,7 +1746,7 @@ DROP INDEX :INDEX_NAME;
17461746
Limit (actual rows=3.00 loops=1)
17471747
-> Merge Join (actual rows=3.00 loops=1)
17481748
Merge Cond: (m2."time" = m1."time")
1749-
Join Filter: (m2.device_id = m1.device_id)
1749+
Join Filter: (m1.device_id = m2.device_id)
17501750
Rows Removed by Join Filter: 4
17511751
-> Custom Scan (ChunkAppend) on join_limit m2 (actual rows=3.00 loops=1)
17521752
Order: m2."time", m2.device_id

test/expected/append-17.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1741,7 +1741,7 @@ DROP INDEX :INDEX_NAME;
17411741
Limit (actual rows=3.00 loops=1)
17421742
-> Merge Join (actual rows=3.00 loops=1)
17431743
Merge Cond: (m2."time" = m1."time")
1744-
Join Filter: (m2.device_id = m1.device_id)
1744+
Join Filter: (m1.device_id = m2.device_id)
17451745
Rows Removed by Join Filter: 4
17461746
-> Custom Scan (ChunkAppend) on join_limit m2 (actual rows=3.00 loops=1)
17471747
Order: m2."time", m2.device_id

test/expected/append-18.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1744,7 +1744,7 @@ DROP INDEX :INDEX_NAME;
17441744
Limit (actual rows=3.00 loops=1)
17451745
-> Merge Join (actual rows=3.00 loops=1)
17461746
Merge Cond: (m2."time" = m1."time")
1747-
Join Filter: (m2.device_id = m1.device_id)
1747+
Join Filter: (m1.device_id = m2.device_id)
17481748
Rows Removed by Join Filter: 4
17491749
-> Custom Scan (ChunkAppend) on join_limit m2 (actual rows=3.00 loops=1)
17501750
Order: m2."time", m2.device_id

tsl/src/nodes/skip_scan/planner.c

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1188,41 +1188,40 @@ static Var *
11881188
get_distinct_var(PlannerInfo *root, Expr *tlexpr, IndexPath *index_path, Path *child_path,
11891189
SkipKeyInfo *skinfo)
11901190
{
1191-
RelOptInfo *rel = child_path->parent;
1191+
RelOptInfo *chunk_rel = child_path->parent;
11921192
RelOptInfo *indexed_rel = index_path->path.parent;
11931193

11941194
Assert(tlexpr && IsA(tlexpr, Var));
11951195
Var *var = castNode(Var, tlexpr);
11961196

11971197
RangeTblEntry *ht_rte = planner_rt_fetch(var->varno, root);
11981198

1199-
/* check whether a skip var is declared NOT NULL
1200-
* it's enough to check hypertable for NOT NULL
1201-
* as NOT NULL constraint will be propagated to and checked on all chunks
1199+
/*
1200+
* Check whether a skip var is declared NOT NULL. It's enough to check
1201+
* hypertable for NOT NULL, because the NOT NULL constraint will be
1202+
* propagated to and checked on all chunks. Postgres doesn't set
1203+
* RelOptInfo.notnullattnums for hypertable because it's an inheritance
1204+
* parent, so check it against the catalog.
12021205
*/
1203-
#if PG17_LT
12041206
skinfo->notnull = ts_get_attnotnull(ht_rte->relid, var->varattno);
1205-
#else
1206-
RelOptInfo *baserel = ((Index) var->varno == rel->relid ? rel : rel->parent);
1207-
skinfo->notnull = bms_is_member(var->varattno, baserel->notnullattnums);
1208-
#endif
12091207

12101208
/* If we are dealing with a hypertable Var extracted from distinctClause will point to
12111209
* the parent hypertable while the IndexPath will be on a Chunk.
12121210
* For a normal PG table they point to the same relation and we are done here. */
1213-
if ((Index) var->varno == rel->relid)
1211+
if ((Index) var->varno == chunk_rel->relid)
12141212
{
12151213
/* Get attribute number for distinct column on a normal PG table */
12161214
skinfo->indexed_column_attno = var->varattno;
12171215
return var;
12181216
}
12191217

1220-
RangeTblEntry *chunk_rte = planner_rt_fetch(rel->relid, root);
1218+
RangeTblEntry *chunk_rte = planner_rt_fetch(chunk_rel->relid, root);
12211219
RangeTblEntry *indexed_rte =
1222-
(indexed_rel == rel ? chunk_rte : planner_rt_fetch(indexed_rel->relid, root));
1220+
(indexed_rel == chunk_rel ? chunk_rte : planner_rt_fetch(indexed_rel->relid, root));
12231221

12241222
/* Check for hypertable */
1225-
if (!ts_is_hypertable(ht_rte->relid) || !bms_is_member(var->varno, rel->top_parent_relids))
1223+
if (!ts_is_hypertable(ht_rte->relid) ||
1224+
!bms_is_member(var->varno, chunk_rel->top_parent_relids))
12261225
{
12271226
return NULL;
12281227
}
@@ -1248,7 +1247,7 @@ get_distinct_var(PlannerInfo *root, Expr *tlexpr, IndexPath *index_path, Path *c
12481247
skinfo->indexed_column_attno = var->varattno;
12491248
}
12501249

1251-
var->varno = rel->relid;
1250+
var->varno = chunk_rel->relid;
12521251

12531252
return var;
12541253
}

tsl/test/expected/transparent_decompression_ordered_index-16.out

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ GROUP BY device_id;
424424
GroupAggregate (actual rows=1.00 loops=1)
425425
Group Key: mt.device_id
426426
-> Nested Loop (actual rows=48.00 loops=1)
427-
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (nd.node = mt.device_id))
427+
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (mt.device_id = nd.node))
428428
Rows Removed by Join Filter: 1493
429429
-> Merge Append (actual rows=1541.00 loops=1)
430430
Sort Key: mt.device_id
@@ -452,7 +452,7 @@ WHERE mt.time > nd.start_time
452452
ORDER BY time;
453453
--- QUERY PLAN ---
454454
Nested Loop (actual rows=48.00 loops=1)
455-
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (nd.node = mt.device_id))
455+
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (mt.device_id = nd.node))
456456
Rows Removed by Join Filter: 1493
457457
-> Custom Scan (ChunkAppend) on metrics_ordered_idx mt (actual rows=1541.00 loops=1)
458458
Order: mt."time"
@@ -1139,7 +1139,7 @@ ORDER BY "time", mt.device_id limit 5;
11391139
--- QUERY PLAN ---
11401140
Limit (actual rows=5.00 loops=1)
11411141
-> Nested Loop (actual rows=5.00 loops=1)
1142-
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (nd.node = mt.device_id))
1142+
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (mt.device_id = nd.node))
11431143
Rows Removed by Join Filter: 1488
11441144
-> Custom Scan (ChunkAppend) on metrics_ordered_idx mt (actual rows=1493.00 loops=1)
11451145
Order: mt."time", mt.device_id
@@ -1184,7 +1184,7 @@ ORDER BY "time", mt.device_id limit 5;
11841184
--- QUERY PLAN ---
11851185
Limit (actual rows=5.00 loops=1)
11861186
-> Nested Loop (actual rows=5.00 loops=1)
1187-
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (nd.node = mt.device_id))
1187+
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (mt.device_id = nd.node))
11881188
Rows Removed by Join Filter: 1488
11891189
-> Custom Scan (ChunkAppend) on metrics_ordered_idx mt (actual rows=1493.00 loops=1)
11901190
Order: mt."time", mt.device_id

tsl/test/expected/transparent_decompression_ordered_index-17.out

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ GROUP BY device_id;
424424
GroupAggregate (actual rows=1.00 loops=1)
425425
Group Key: mt.device_id
426426
-> Nested Loop (actual rows=48.00 loops=1)
427-
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (nd.node = mt.device_id))
427+
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (mt.device_id = nd.node))
428428
Rows Removed by Join Filter: 1493
429429
-> Merge Append (actual rows=1541.00 loops=1)
430430
Sort Key: mt.device_id
@@ -452,7 +452,7 @@ WHERE mt.time > nd.start_time
452452
ORDER BY time;
453453
--- QUERY PLAN ---
454454
Nested Loop (actual rows=48.00 loops=1)
455-
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (nd.node = mt.device_id))
455+
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (mt.device_id = nd.node))
456456
Rows Removed by Join Filter: 1493
457457
-> Custom Scan (ChunkAppend) on metrics_ordered_idx mt (actual rows=1541.00 loops=1)
458458
Order: mt."time"
@@ -1139,7 +1139,7 @@ ORDER BY "time", mt.device_id limit 5;
11391139
--- QUERY PLAN ---
11401140
Limit (actual rows=5.00 loops=1)
11411141
-> Nested Loop (actual rows=5.00 loops=1)
1142-
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (nd.node = mt.device_id))
1142+
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (mt.device_id = nd.node))
11431143
Rows Removed by Join Filter: 1488
11441144
-> Custom Scan (ChunkAppend) on metrics_ordered_idx mt (actual rows=1493.00 loops=1)
11451145
Order: mt."time", mt.device_id
@@ -1184,7 +1184,7 @@ ORDER BY "time", mt.device_id limit 5;
11841184
--- QUERY PLAN ---
11851185
Limit (actual rows=5.00 loops=1)
11861186
-> Nested Loop (actual rows=5.00 loops=1)
1187-
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (nd.node = mt.device_id))
1187+
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (mt.device_id = nd.node))
11881188
Rows Removed by Join Filter: 1488
11891189
-> Custom Scan (ChunkAppend) on metrics_ordered_idx mt (actual rows=1493.00 loops=1)
11901190
Order: mt."time", mt.device_id

tsl/test/expected/transparent_decompression_ordered_index-18.out

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1148,7 +1148,7 @@ ORDER BY "time", mt.device_id limit 5;
11481148
--- QUERY PLAN ---
11491149
Limit (actual rows=5.00 loops=1)
11501150
-> Nested Loop (actual rows=5.00 loops=1)
1151-
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (nd.node = mt.device_id))
1151+
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (mt.device_id = nd.node))
11521152
Rows Removed by Join Filter: 1488
11531153
-> Custom Scan (ChunkAppend) on metrics_ordered_idx mt (actual rows=1493.00 loops=1)
11541154
Order: mt."time", mt.device_id
@@ -1193,7 +1193,7 @@ ORDER BY "time", mt.device_id limit 5;
11931193
--- QUERY PLAN ---
11941194
Limit (actual rows=5.00 loops=1)
11951195
-> Nested Loop (actual rows=5.00 loops=1)
1196-
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (nd.node = mt.device_id))
1196+
Join Filter: ((mt."time" > nd.start_time) AND (mt."time" < nd.stop_time) AND (mt.device_id = nd.node))
11971197
Rows Removed by Join Filter: 1488
11981198
-> Custom Scan (ChunkAppend) on metrics_ordered_idx mt (actual rows=1493.00 loops=1)
11991199
Order: mt."time", mt.device_id

0 commit comments

Comments
 (0)