diff options
| author | Tom Lane <tgl@sss.pgh.pa.us> | 2023-12-18 12:46:07 -0500 |
|---|---|---|
| committer | Tom Lane <tgl@sss.pgh.pa.us> | 2023-12-18 12:46:10 -0500 |
| commit | 8b965c549dc8753be8a38c4a1b9fabdb535a4338 (patch) | |
| tree | 19413713d999928124eca96353a9dbbd2ff84f6e | |
| parent | 64b1fb5f03266f0ef4eef3ad2b7d97170bb05b78 (diff) | |
| download | postgresql-8b965c549dc8753be8a38c4a1b9fabdb535a4338.tar.gz postgresql-8b965c549dc8753be8a38c4a1b9fabdb535a4338.zip | |
compute_bitmap_pages' loop_count parameter should be double not int.
The value was double in the original implementation of this logic.
Commit da08a6598 pulled it out into a subroutine, but carelessly
declared the parameter as int when it should have been double.
On most platforms, the only ill effect would be to clamp the value
to be not more than INT_MAX, which would seldom be exceeded and
probably wouldn't change the estimates too much anyway. Nonetheless,
it's wrong and can cause complaints from ubsan.
While here, improve the comments and parameter names.
This is an ABI change in a globally exposed subroutine, so
back-patching would create some risk of breaking extensions.
The value of the fix doesn't seem high enough to warrant taking
that risk, so fix in HEAD only.
Per report from Alexander Lakhin.
Discussion: https://postgr.es/m/f5e15fe1-202d-1936-f47c-f0c69a936b72@gmail.com
| -rw-r--r-- | src/backend/optimizer/path/costsize.c | 22 | ||||
| -rw-r--r-- | src/include/optimizer/cost.h | 3 |
2 files changed, 17 insertions, 8 deletions
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index d6ceafd51c5..5227346aeb1 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -6393,12 +6393,20 @@ get_parallel_divisor(Path *path) /* * compute_bitmap_pages + * Estimate number of pages fetched from heap in a bitmap heap scan. * - * compute number of pages fetched from heap in bitmap heap scan. + * 'baserel' is the relation to be scanned + * 'bitmapqual' is a tree of IndexPaths, BitmapAndPaths, and BitmapOrPaths + * 'loop_count' is the number of repetitions of the indexscan to factor into + * estimates of caching behavior + * + * If cost_p isn't NULL, the indexTotalCost estimate is returned in *cost_p. + * If tuples_p isn't NULL, the tuples_fetched estimate is returned in *tuples_p. */ double -compute_bitmap_pages(PlannerInfo *root, RelOptInfo *baserel, Path *bitmapqual, - int loop_count, Cost *cost, double *tuple) +compute_bitmap_pages(PlannerInfo *root, RelOptInfo *baserel, + Path *bitmapqual, double loop_count, + Cost *cost_p, double *tuples_p) { Cost indexTotalCost; Selectivity indexSelectivity; @@ -6488,10 +6496,10 @@ compute_bitmap_pages(PlannerInfo *root, RelOptInfo *baserel, Path *bitmapqual, (lossy_pages / heap_pages) * baserel->tuples); } - if (cost) - *cost = indexTotalCost; - if (tuple) - *tuple = tuples_fetched; + if (cost_p) + *cost_p = indexTotalCost; + if (tuples_p) + *tuples_p = tuples_fetched; return pages_fetched; } diff --git a/src/include/optimizer/cost.h b/src/include/optimizer/cost.h index 6d50afbf74c..366adbfc39e 100644 --- a/src/include/optimizer/cost.h +++ b/src/include/optimizer/cost.h @@ -210,6 +210,7 @@ extern void set_result_size_estimates(PlannerInfo *root, RelOptInfo *rel); extern void set_foreign_size_estimates(PlannerInfo *root, RelOptInfo *rel); extern PathTarget *set_pathtarget_cost_width(PlannerInfo *root, PathTarget *target); extern double compute_bitmap_pages(PlannerInfo *root, RelOptInfo *baserel, - Path *bitmapqual, int loop_count, Cost *cost, double *tuple); + Path *bitmapqual, double loop_count, + Cost *cost_p, double *tuples_p); #endif /* COST_H */ |