Skip to content

Commit

Permalink
Use cached Chunk struct when considering compressed paths
Browse files Browse the repository at this point in the history
Full catalog lookups for a Chunk are expensive, avoiding them speeds up
the planning.
  • Loading branch information
akuzm committed Aug 15, 2023
1 parent fb617e4 commit 04ce1bc
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/planner/expand_hypertable.c
Original file line number Diff line number Diff line change
Expand Up @@ -1507,7 +1507,7 @@ ts_plan_expand_hypertable_chunks(Hypertable *ht, PlannerInfo *root, RelOptInfo *
#endif
}

ts_get_private_reloptinfo(child_rel)->chunk = chunks[i];
ts_get_private_reloptinfo(child_rel)->cached_chunk_struct = chunks[i];
Assert(chunks[i]->table_id == root->simple_rte_array[child_rtindex]->relid);
}
}
Expand Down
13 changes: 6 additions & 7 deletions src/planner/planner.c
Original file line number Diff line number Diff line change
Expand Up @@ -1096,18 +1096,17 @@ apply_optimizations(PlannerInfo *root, TsRelType reltype, RelOptInfo *rel, Range
case TS_REL_CHUNK_STANDALONE:
case TS_REL_CHUNK_CHILD:
ts_sort_transform_optimization(root, rel);
/*
* Since the sort optimization adds new paths to the rel it has
* to happen before any optimizations that replace pathlist.
*/
if (ts_cm_functions->set_rel_pathlist_query != NULL)
ts_cm_functions->set_rel_pathlist_query(root, rel, rel->relid, rte, ht);
break;
default:
break;
}

/*
* Since the sort optimization adds new paths to the rel it has
* to happen before any optimizations that replace pathlist.
*/
if (ts_cm_functions->set_rel_pathlist_query != NULL)
ts_cm_functions->set_rel_pathlist_query(root, rel, rel->relid, rte, ht);

if (reltype == TS_REL_HYPERTABLE &&
#if PG14_GE
(root->parse->commandType == CMD_SELECT || root->parse->commandType == CMD_DELETE ||
Expand Down
2 changes: 1 addition & 1 deletion src/planner/planner.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ typedef struct TimescaleDBPrivate
TsFdwRelInfo *fdw_relation_info;

/* Cached chunk data for the chunk relinfo. */
Chunk *chunk;
Chunk *cached_chunk_struct;
} TimescaleDBPrivate;

extern TSDLLEXPORT bool ts_rte_is_hypertable(const RangeTblEntry *rte, bool *isdistributed);
Expand Down
4 changes: 2 additions & 2 deletions tsl/src/fdw/data_node_chunk_assignment.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ data_node_chunk_assignment_assign_chunk(DataNodeChunkAssignments *scas, RelOptIn
*/
Oid remote_chunk_relid = InvalidOid;
ListCell *lc;
foreach (lc, chunk_private->chunk->data_nodes)
foreach (lc, chunk_private->cached_chunk_struct->data_nodes)
{
ChunkDataNode *cdn = (ChunkDataNode *) lfirst(lc);
if (cdn->foreign_server_oid == chunkrel->serverid)
Expand All @@ -94,7 +94,7 @@ data_node_chunk_assignment_assign_chunk(DataNodeChunkAssignments *scas, RelOptIn
*/
old = MemoryContextSwitchTo(scas->mctx);
sca->chunk_relids = bms_add_member(sca->chunk_relids, chunkrel->relid);
sca->chunks = lappend(sca->chunks, chunk_private->chunk);
sca->chunks = lappend(sca->chunks, chunk_private->cached_chunk_struct);
sca->remote_chunk_ids = lappend_int(sca->remote_chunk_ids, remote_chunk_relid);
sca->pages += chunkrel->pages;
sca->rows += chunkrel->rows;
Expand Down
7 changes: 4 additions & 3 deletions tsl/src/fdw/relinfo.c
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,10 @@ estimate_chunk_size(PlannerInfo *root, RelOptInfo *chunk_rel)
* exclusion, so we'll have to look up this info now.
*/
TimescaleDBPrivate *chunk_private = ts_get_private_reloptinfo(chunk_rel);
if (chunk_private->chunk == NULL)
if (chunk_private->cached_chunk_struct == NULL)
{
RangeTblEntry *chunk_rte = planner_rt_fetch(chunk_rel->relid, root);
chunk_private->chunk =
chunk_private->cached_chunk_struct =
ts_chunk_get_by_relid(chunk_rte->relid, true /* fail_if_not_found */);
}

Expand All @@ -319,7 +319,8 @@ estimate_chunk_size(PlannerInfo *root, RelOptInfo *chunk_rel)
Hypertable *ht = ts_hypertable_cache_get_entry(hcache, parent_rte->relid, CACHE_FLAG_NONE);
Hyperspace *hyperspace = ht->space;

const double fillfactor = estimate_chunk_fillfactor(chunk_private->chunk, hyperspace);
const double fillfactor =
estimate_chunk_fillfactor(chunk_private->cached_chunk_struct, hyperspace);

/* Can't have nonzero tuples in zero pages */
Assert(parent_private->average_chunk_pages != 0 || parent_private->average_chunk_tuples <= 0);
Expand Down
31 changes: 26 additions & 5 deletions tsl/src/planner.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,37 @@ tsl_set_rel_pathlist_query(PlannerInfo *root, RelOptInfo *rel, Index rti, RangeT
* We check if it is the ONLY case by calling ts_rte_is_marked_for_expansion.
* Respecting ONLY here is important to not break postgres tools like pg_dump.
*/
TimescaleDBPrivate *fdw_private = (TimescaleDBPrivate *) rel->fdw_private;
if (ts_guc_enable_transparent_decompression && ht &&
(rel->reloptkind == RELOPT_OTHER_MEMBER_REL ||
(rel->reloptkind == RELOPT_BASEREL && ts_rte_is_marked_for_expansion(rte))) &&
TS_HYPERTABLE_HAS_COMPRESSION_TABLE(ht) && rel->fdw_private != NULL &&
((TimescaleDBPrivate *) rel->fdw_private)->compressed)
TS_HYPERTABLE_HAS_COMPRESSION_TABLE(ht) && fdw_private != NULL && fdw_private->compressed)
{
Chunk *chunk = ts_chunk_get_by_relid(rte->relid, true);
if (fdw_private->cached_chunk_struct == NULL)
{
/*
* We can not have the cached Chunk struct,
* 1) if it was a direct query on the chunk;
* 2) if it is not a SELECT QUERY.
* Caching is done by our hypertable expansion, which doesn't run in
* these cases.
*
* Also on PG13 when a DELETE query runs through SPI, its command
* type is CMD_SELECT. Apparently it goes into inheritance_planner,
* which uses a hack to pretend it's actually a SELECT query, but
* for some reason for non-SPI queries the query type is still
* correct. You can observe it in the continuous_aggs-13 test.
* Just ignore this assertion on 13 and look up the chunk.
*/
#if PG14_GE
Assert(rel->reloptkind == RELOPT_BASEREL || root->parse->commandType != CMD_SELECT);
#endif
fdw_private->cached_chunk_struct =
ts_chunk_get_by_relid(rte->relid, /* fail_if_not_found = */ true);
}

if (chunk->fd.compressed_chunk_id != INVALID_CHUNK_ID)
ts_decompress_chunk_generate_paths(root, rel, ht, chunk);
if (fdw_private->cached_chunk_struct->fd.compressed_chunk_id != INVALID_CHUNK_ID)
ts_decompress_chunk_generate_paths(root, rel, ht, fdw_private->cached_chunk_struct);
}
}

Expand Down

0 comments on commit 04ce1bc

Please sign in to comment.