Skip to content

Commit d343ca5

Browse files
authored
Propagate consider_startup flag to compressed chunk table (#9738)
It seems at the moment it's just not set, which might lead to suboptimal plan choice. We also have an heuristic to trigger it for queries that might need SkipScan, so move this to happen earlier. Also update it to work for multicolumn DISTINCT. The flag propagation itself doesn't influence anything at the moment, because the Index paths are still kept based on pathkeys. But logically it's more correct. Disable-check: force-changelog-file
1 parent a93f871 commit d343ca5

2 files changed

Lines changed: 28 additions & 21 deletions

File tree

tsl/src/nodes/columnar_scan/columnar_scan.c

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1175,6 +1175,27 @@ ts_columnar_scan_generate_paths(PlannerInfo *root, RelOptInfo *chunk_rel, const
11751175
chunk_rel->pathlist = NIL;
11761176
chunk_rel->partial_pathlist = NIL;
11771177

1178+
/*
1179+
* We want to consider startup costs so that IndexScan is preferred to
1180+
* sorted SeqScan when we may have a chance to use SkipScan. We consider
1181+
* startup costs for LIMIT queries, and SkipScan is basically a
1182+
* "LIMIT 1" query run "ndistinct" times. At this point we don't have
1183+
* all information to check if SkipScan can be used, but we can narrow
1184+
* it down.
1185+
*
1186+
* First, check if this query is candidate for SELECT DISTINCT SkipScan.
1187+
*/
1188+
const bool potential_select_distinct = list_length(root->distinct_pathkeys) >= 1;
1189+
1190+
/* Next, candidate for DISTINCT aggregate SkipScan */
1191+
const bool potential_distinct_aggregate =
1192+
root->numOrderedAggs >= 1 && list_length(root->group_pathkeys) == 1;
1193+
1194+
if (potential_select_distinct || potential_distinct_aggregate)
1195+
{
1196+
chunk_rel->consider_startup = true;
1197+
}
1198+
11781199
/* add RangeTblEntry and RelOptInfo for compressed chunk */
11791200
columnar_scan_add_plannerinfo(root,
11801201
compression_info,
@@ -1193,8 +1214,6 @@ ts_columnar_scan_generate_paths(PlannerInfo *root, RelOptInfo *chunk_rel, const
11931214

11941215
RelOptInfo *compressed_rel = compression_info->compressed_rel;
11951216

1196-
compressed_rel->consider_parallel = chunk_rel->consider_parallel;
1197-
11981217
/* translate chunk_rel->baserestrictinfo */
11991218
if (ts_guc_enable_columnar_scan_filter_pushdown)
12001219
{
@@ -1267,25 +1286,6 @@ ts_columnar_scan_generate_paths(PlannerInfo *root, RelOptInfo *chunk_rel, const
12671286
&sort_info,
12681287
compression_info);
12691288

1270-
/*
1271-
* We want to consider startup costs so that IndexScan is preferred to
1272-
* sorted SeqScan when we may have a chance to use SkipScan. We consider
1273-
* startup costs for LIMIT queries, and SkipScan is basically a
1274-
* "LIMIT 1" query run "ndistinct" times. At this point we don't have
1275-
* all information to check if SkipScan can be used, but we can narrow
1276-
* it down.
1277-
*/
1278-
if (!chunk_rel->consider_startup && IsA(compressed_path, IndexPath))
1279-
{
1280-
/* Candidate for SELECT DISTINCT SkipScan */
1281-
if (list_length(root->distinct_pathkeys) == 1
1282-
/* Candidate for DISTINCT aggregate SkipScan */
1283-
|| (root->numOrderedAggs >= 1 && list_length(root->group_pathkeys) == 1))
1284-
{
1285-
chunk_rel->consider_startup = true;
1286-
}
1287-
}
1288-
12891289
/*
12901290
* Add the paths to the chunk relation.
12911291
*/
@@ -2404,6 +2404,9 @@ columnar_scan_add_plannerinfo(PlannerInfo *root, CompressionInfo *info, const Ch
24042404
/* translate chunk_rel->joininfo for compressed_rel */
24052405
compressed_rel_setup_joininfo(compressed_rel, info);
24062406

2407+
compressed_rel->consider_parallel = chunk_rel->consider_parallel;
2408+
compressed_rel->consider_startup = chunk_rel->consider_startup;
2409+
24072410
/*
24082411
* Force parallel plan creation, see compute_parallel_worker().
24092412
* This is not compatible with ts_classify_relation(), but on the other hand

tsl/test/expected/skip_scan.out

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2878,6 +2878,10 @@ psql:include/skip_scan_multi_query.sql:44: INFO: SkipScan used on compress_hype
28782878
psql:include/skip_scan_multi_query.sql:44: INFO: SkipScan used on compress_hyper_9_76_chunk_status_region_dev_dev_name__ts_me_idx(region NOT NULL, dev NOT NULL)
28792879
-- RowCompareExpr
28802880
:PREFIX SELECT DISTINCT ON (status, dev) * FROM :TABLE WHERE region = 'reg_1' and (status, dev) > (1,2);
2881+
psql:include/skip_scan_multi_query.sql:47: INFO: SkipScan used on compress_hyper_9_73_chunk_status_region_dev_dev_name__ts_me_idx(status NOT NULL, dev NOT NULL)
2882+
psql:include/skip_scan_multi_query.sql:47: INFO: SkipScan used on compress_hyper_9_74_chunk_status_region_dev_dev_name__ts_me_idx(status NOT NULL, dev NOT NULL)
2883+
psql:include/skip_scan_multi_query.sql:47: INFO: SkipScan used on compress_hyper_9_75_chunk_status_region_dev_dev_name__ts_me_idx(status NOT NULL, dev NOT NULL)
2884+
psql:include/skip_scan_multi_query.sql:47: INFO: SkipScan used on compress_hyper_9_76_chunk_status_region_dev_dev_name__ts_me_idx(status NOT NULL, dev NOT NULL)
28812885
-- always false expr similar to our initial skip qual
28822886
:PREFIX SELECT DISTINCT ON (status, region, dev) * FROM :TABLE WHERE dev > NULL and status > NULL and region > NULL;
28832887
-- no tuples matching

0 commit comments

Comments
 (0)