Skip to content

Commit 500f646

Browse files
author
Richard Guo
committed
Fix assertion failure in generate_orderedappend_paths()
In generate_orderedappend_paths(), there is an assumption that a child relation's row estimate is always greater than zero. There is an Assert verifying this assumption, and the estimate is also used to convert an absolute tuple count into a fraction. However, this assumption is not always valid -- for example, upper relations can have their row estimates unset, resulting in a value of zero. This can cause an assertion failure in debug builds or lead to the tuple fraction being computed as infinity in production builds. To fix, use the row estimate from the cheapest_total path to compute the tuple fraction. The row estimate in this path should already have been forced to a valid value. In passing, update the comment for generate_orderedappend_paths() to note that the function also considers the cheapest-fractional case when not all tuples need to be retrieved. That is, it collects all the cheapest fractional paths and builds an ordered append path for each interesting ordering. Backpatch to v18, where this issue was introduced. Bug: #19102 Reported-by: Kuntal Ghosh <kuntalghosh.2007@gmail.com> Author: Richard Guo <guofenglinux@gmail.com> Reviewed-by: Kuntal Ghosh <kuntalghosh.2007@gmail.com> Reviewed-by: Andrei Lepikhov <lepihov@gmail.com> Discussion: https://postgr.es/m/19102-93480667e1200169@postgresql.org Backpatch-through: 18
1 parent d09047d commit 500f646

File tree

3 files changed

+49
-7
lines changed

3 files changed

+49
-7
lines changed

src/backend/optimizer/path/allpaths.c

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1727,9 +1727,11 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
17271727
* We generate a path for each ordering (pathkey list) appearing in
17281728
* all_child_pathkeys.
17291729
*
1730-
* We consider both cheapest-startup and cheapest-total cases, ie, for each
1731-
* interesting ordering, collect all the cheapest startup subpaths and all the
1732-
* cheapest total paths, and build a suitable path for each case.
1730+
* We consider the cheapest-startup and cheapest-total cases, and also the
1731+
* cheapest-fractional case when not all tuples need to be retrieved. For each
1732+
* interesting ordering, we collect all the cheapest startup subpaths, all the
1733+
* cheapest total paths, and, if applicable, all the cheapest fractional paths,
1734+
* and build a suitable path for each case.
17331735
*
17341736
* We don't currently generate any parameterized ordered paths here. While
17351737
* it would not take much more code here to do so, it's very unclear that it
@@ -1894,14 +1896,18 @@ generate_orderedappend_paths(PlannerInfo *root, RelOptInfo *rel,
18941896
double path_fraction = root->tuple_fraction;
18951897

18961898
/*
1897-
* Merge Append considers only live children relations. Dummy
1898-
* relations must be filtered out before.
1899+
* We should not have a dummy child relation here. However,
1900+
* we cannot use childrel->rows to compute the tuple fraction,
1901+
* as childrel can be an upper relation with an unset row
1902+
* estimate. Instead, we use the row estimate from the
1903+
* cheapest_total path, which should already have been forced
1904+
* to a sane value.
18991905
*/
1900-
Assert(childrel->rows > 0);
1906+
Assert(cheapest_total->rows > 0);
19011907

19021908
/* Convert absolute limit to a path fraction */
19031909
if (path_fraction >= 1.0)
1904-
path_fraction /= childrel->rows;
1910+
path_fraction /= cheapest_total->rows;
19051911

19061912
cheapest_fractional =
19071913
get_cheapest_fractional_path_for_pathkeys(childrel->pathlist,

src/test/regress/expected/partition_aggregate.out

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,37 @@ SELECT a FROM pagg_tab WHERE a < 3 GROUP BY a ORDER BY 1;
335335
2
336336
(3 rows)
337337

338+
-- Test partitionwise aggregation with ordered append path built from fractional paths
339+
EXPLAIN (COSTS OFF)
340+
SELECT count(*) FROM pagg_tab GROUP BY c ORDER BY c LIMIT 1;
341+
QUERY PLAN
342+
------------------------------------------------------------
343+
Limit
344+
-> Merge Append
345+
Sort Key: pagg_tab.c
346+
-> GroupAggregate
347+
Group Key: pagg_tab.c
348+
-> Sort
349+
Sort Key: pagg_tab.c
350+
-> Seq Scan on pagg_tab_p1 pagg_tab
351+
-> GroupAggregate
352+
Group Key: pagg_tab_1.c
353+
-> Sort
354+
Sort Key: pagg_tab_1.c
355+
-> Seq Scan on pagg_tab_p2 pagg_tab_1
356+
-> GroupAggregate
357+
Group Key: pagg_tab_2.c
358+
-> Sort
359+
Sort Key: pagg_tab_2.c
360+
-> Seq Scan on pagg_tab_p3 pagg_tab_2
361+
(18 rows)
362+
363+
SELECT count(*) FROM pagg_tab GROUP BY c ORDER BY c LIMIT 1;
364+
count
365+
-------
366+
250
367+
(1 row)
368+
338369
RESET enable_hashagg;
339370
-- ROLLUP, partitionwise aggregation does not apply
340371
EXPLAIN (COSTS OFF)

src/test/regress/sql/partition_aggregate.sql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ EXPLAIN (COSTS OFF)
7474
SELECT a FROM pagg_tab WHERE a < 3 GROUP BY a ORDER BY 1;
7575
SELECT a FROM pagg_tab WHERE a < 3 GROUP BY a ORDER BY 1;
7676

77+
-- Test partitionwise aggregation with ordered append path built from fractional paths
78+
EXPLAIN (COSTS OFF)
79+
SELECT count(*) FROM pagg_tab GROUP BY c ORDER BY c LIMIT 1;
80+
SELECT count(*) FROM pagg_tab GROUP BY c ORDER BY c LIMIT 1;
81+
7782
RESET enable_hashagg;
7883

7984
-- ROLLUP, partitionwise aggregation does not apply

0 commit comments

Comments
 (0)