From b46cd2f88ead9a6facf89064cc2ac2f7aac01134 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 19 Nov 2024 20:35:29 -0500 Subject: [PATCH v2 3/5] Get rid of choose_hashed_setop(). This logic is a hangover from days when we could only generate one plan per setop node. Now, it's better to generate multiple Paths and let add_path choose the cheapest. The main advantage is not having to keep choose_hashed_setop in perfect sync with cost estimates made elsewhere --- and indeed, it looks like it was out of sync already. Not to mention that it doesn't account for presorted input, which doesn't matter too much right now but will soon. This does require fixing create_setop_path to generate good cost estimates. That was another area where we'd not maintained things well, because it wasn't distinguishing sorted from hashed costs at all. With the formulas I chose, the total cost attributed to the node is actually the same for sorted and hashed modes --- but the startup cost is wildly different, because hashed mode does basically all the work before returning anything. I also modified the existing logic that disables hashed setops so that it works by incrementing path->disabled_nodes rather than not generating the path at all. That seems more in keeping with the methods established by commit e22253467. (But should we put that logic in create_setop_path, rather than its caller?) This results in two changes in plans that are visible in the regression tests. The one in union.sql seems fine: it's now preferring sorted over hashed mode for a zero-column INTERSECT, since no sort is really required. The one in subselect.sql is trying to test nodeSetOp's rescan logic, so I extended it to test both sorted and hashed modes. --- src/backend/optimizer/prep/prepunion.c | 226 +++++++++--------------- src/backend/optimizer/util/pathnode.c | 50 +++++- src/test/regress/expected/subselect.out | 48 ++++- src/test/regress/expected/union.out | 2 +- src/test/regress/sql/subselect.sql | 32 +++- 5 files changed, 200 insertions(+), 158 deletions(-) diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 977bd546e0..21513dbb19 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -63,10 +63,6 @@ static List *plan_union_children(PlannerInfo *root, List **tlist_list, List **istrivial_tlist); static void postprocess_setop_rel(PlannerInfo *root, RelOptInfo *rel); -static bool choose_hashed_setop(PlannerInfo *root, List *groupClauses, - Path *lpath, Path *rpath, - double dNumGroups, double dNumOutputRows, - const char *construct); static List *generate_setop_tlist(List *colTypes, List *colCollations, Index varno, bool hack_constants, @@ -1025,7 +1021,8 @@ generate_nonunion_paths(SetOperationStmt *op, PlannerInfo *root, dRightGroups, dNumGroups, dNumOutputRows; - bool use_hash; + bool can_sort; + bool can_hash; SetOpCmd cmd; /* @@ -1130,15 +1127,76 @@ generate_nonunion_paths(SetOperationStmt *op, PlannerInfo *root, dNumGroups = dLeftGroups; dNumOutputRows = op->all ? Min(lpath->rows, rpath->rows) : dNumGroups; } + result_rel->rows = dNumOutputRows; + + /* Select the SetOpCmd type */ + switch (op->op) + { + case SETOP_INTERSECT: + cmd = op->all ? SETOPCMD_INTERSECT_ALL : SETOPCMD_INTERSECT; + break; + case SETOP_EXCEPT: + cmd = op->all ? SETOPCMD_EXCEPT_ALL : SETOPCMD_EXCEPT; + break; + default: + elog(ERROR, "unrecognized set op: %d", (int) op->op); + cmd = SETOPCMD_INTERSECT; /* keep compiler quiet */ + break; + } + + /* Check whether the operators support sorting or hashing */ + can_sort = grouping_is_sortable(groupList); + can_hash = grouping_is_hashable(groupList); + if (!can_sort && !can_hash) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + /* translator: %s is INTERSECT or EXCEPT */ + errmsg("could not implement %s", + (op->op == SETOP_INTERSECT) ? "INTERSECT" : "EXCEPT"), + errdetail("Some of the datatypes only support hashing, while others only support sorting."))); /* - * Decide whether to hash or sort, and add sort nodes if needed. + * If we can hash, that just requires a SetOp atop the cheapest inputs. */ - use_hash = choose_hashed_setop(root, groupList, lpath, rpath, - dNumGroups, dNumOutputRows, - (op->op == SETOP_INTERSECT) ? "INTERSECT" : "EXCEPT"); + if (can_hash) + { + Size hash_mem_limit = get_hash_memory_limit(); + Size hashentrysize; + + path = (Path *) create_setop_path(root, + result_rel, + lpath, + rpath, + cmd, + SETOP_HASHED, + groupList, + dNumGroups, + dNumOutputRows); - if (groupList && !use_hash) + /* + * Mark the path as disabled if enable_hashagg is off. While this + * isn't exactly a HashAgg node, it seems close enough to justify + * letting that switch control it. + */ + if (!enable_hashagg) + path->disabled_nodes++; + + /* + * Also disable if it doesn't look like the hashtable will fit into + * hash_mem. + */ + hashentrysize = MAXALIGN(lpath->pathtarget->width) + + MAXALIGN(SizeofMinimalTupleHeader); + if (hashentrysize * dNumGroups > hash_mem_limit) + path->disabled_nodes++; + + add_path(result_rel, path); + } + + /* + * If we can sort, sort the inputs if needed before adding SetOp. + */ + if (can_sort) { List *pathkeys; @@ -1160,36 +1218,19 @@ generate_nonunion_paths(SetOperationStmt *op, PlannerInfo *root, rpath, pathkeys, -1.0); - } - /* - * Finally, add a SetOp path node to generate the correct output. - */ - switch (op->op) - { - case SETOP_INTERSECT: - cmd = op->all ? SETOPCMD_INTERSECT_ALL : SETOPCMD_INTERSECT; - break; - case SETOP_EXCEPT: - cmd = op->all ? SETOPCMD_EXCEPT_ALL : SETOPCMD_EXCEPT; - break; - default: - elog(ERROR, "unrecognized set op: %d", (int) op->op); - cmd = SETOPCMD_INTERSECT; /* keep compiler quiet */ - break; + path = (Path *) create_setop_path(root, + result_rel, + lpath, + rpath, + cmd, + SETOP_SORTED, + groupList, + dNumGroups, + dNumOutputRows); + add_path(result_rel, path); } - path = (Path *) create_setop_path(root, - result_rel, - lpath, - rpath, - cmd, - use_hash ? SETOP_HASHED : SETOP_SORTED, - groupList, - dNumGroups, - dNumOutputRows); - - result_rel->rows = path->rows; - add_path(result_rel, path); + return result_rel; } @@ -1276,115 +1317,6 @@ postprocess_setop_rel(PlannerInfo *root, RelOptInfo *rel) set_cheapest(rel); } -/* - * choose_hashed_setop - should we use hashing for a set operation? - * - * XXX probably this should go away: just make both paths and let - * add_path sort it out. - */ -static bool -choose_hashed_setop(PlannerInfo *root, List *groupClauses, - Path *lpath, Path *rpath, - double dNumGroups, double dNumOutputRows, - const char *construct) -{ - int numGroupCols = list_length(groupClauses); - Size hash_mem_limit = get_hash_memory_limit(); - bool can_sort; - bool can_hash; - Size hashentrysize; - Path hashed_p; - Path sorted_p; - double tuple_fraction; - - /* Check whether the operators support sorting or hashing */ - can_sort = grouping_is_sortable(groupClauses); - can_hash = grouping_is_hashable(groupClauses); - if (can_hash && can_sort) - { - /* we have a meaningful choice to make, continue ... */ - } - else if (can_hash) - return true; - else if (can_sort) - return false; - else - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - /* translator: %s is UNION, INTERSECT, or EXCEPT */ - errmsg("could not implement %s", construct), - errdetail("Some of the datatypes only support hashing, while others only support sorting."))); - - /* Prefer sorting when enable_hashagg is off */ - if (!enable_hashagg) - return false; - - /* - * Don't do it if it doesn't look like the hashtable will fit into - * hash_mem. - */ - hashentrysize = MAXALIGN(lpath->pathtarget->width) + MAXALIGN(SizeofMinimalTupleHeader); - - if (hashentrysize * dNumGroups > hash_mem_limit) - return false; - - /* - * See if the estimated cost is no more than doing it the other way. - * - * We need to consider input_plan + hashagg versus input_plan + sort + - * group. XXX NOT TRUE: Note that the actual result plan might involve a - * SetOp or Unique node, not Agg or Group, but the cost estimates for Agg - * and Group should be close enough for our purposes here. - * - * These path variables are dummies that just hold cost fields; we don't - * make actual Paths for these steps. - */ - cost_agg(&hashed_p, root, AGG_HASHED, NULL, - numGroupCols, dNumGroups, - NIL, - lpath->disabled_nodes + rpath->disabled_nodes, - lpath->startup_cost + rpath->startup_cost, - lpath->total_cost + rpath->total_cost, - lpath->rows + rpath->rows, - lpath->pathtarget->width); - - /* - * Now for the sorted case. XXX NOT TRUE: Note that the input is *always* - * unsorted, since it was made by appending unrelated sub-relations - * together. - */ - sorted_p.disabled_nodes = lpath->disabled_nodes + rpath->disabled_nodes; - sorted_p.startup_cost = lpath->startup_cost + rpath->startup_cost; - sorted_p.total_cost = lpath->total_cost + rpath->total_cost; - /* XXX cost_sort doesn't actually look at pathkeys, so just pass NIL */ - cost_sort(&sorted_p, root, NIL, sorted_p.disabled_nodes, - sorted_p.total_cost, - lpath->rows + rpath->rows, - lpath->pathtarget->width, - 0.0, work_mem, -1.0); - cost_group(&sorted_p, root, numGroupCols, dNumGroups, - NIL, - sorted_p.disabled_nodes, - sorted_p.startup_cost, sorted_p.total_cost, - lpath->rows + rpath->rows); - - /* - * Now make the decision using the top-level tuple fraction. First we - * have to convert an absolute count (LIMIT) into fractional form. - */ - tuple_fraction = root->tuple_fraction; - if (tuple_fraction >= 1.0) - tuple_fraction /= dNumOutputRows; - - if (compare_fractional_path_costs(&hashed_p, &sorted_p, - tuple_fraction) < 0) - { - /* Hashed is cheaper, so use it */ - return true; - } - return false; -} - /* * Generate targetlist for a set-operation plan node * diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c index e52e4b1d67..d7422f2dd8 100644 --- a/src/backend/optimizer/util/pathnode.c +++ b/src/backend/optimizer/util/pathnode.c @@ -3681,17 +3681,51 @@ create_setop_path(PlannerInfo *root, pathnode->numGroups = numGroups; /* - * Charge one cpu_operator_cost per comparison per input tuple. We assume - * all columns get compared at most of the tuples. - * - * XXX all wrong for hashing + * Compute cost estimates. As things stand, we end up with the same total + * cost in this node for sort and hash methods, but different startup + * costs. This could be refined perhaps, but it'll do for now. */ pathnode->path.disabled_nodes = leftpath->disabled_nodes + rightpath->disabled_nodes; - pathnode->path.startup_cost = - leftpath->startup_cost + rightpath->startup_cost; - pathnode->path.total_cost = leftpath->total_cost + rightpath->total_cost + - cpu_operator_cost * (leftpath->rows + rightpath->rows) * list_length(groupList); + if (strategy == SETOP_SORTED) + { + /* + * In sorted mode, we can emit output incrementally. Charge one + * cpu_operator_cost per comparison per input tuple. Like cost_group, + * we assume all columns get compared at most of the tuples. + */ + pathnode->path.startup_cost = + leftpath->startup_cost + rightpath->startup_cost; + pathnode->path.total_cost = + leftpath->total_cost + rightpath->total_cost + + cpu_operator_cost * (leftpath->rows + rightpath->rows) * list_length(groupList); + + /* + * Also charge a small amount per extracted tuple. Like cost_sort, + * charge only operator cost not cpu_tuple_cost, since SetOp does no + * qual-checking or projection. + */ + pathnode->path.total_cost += cpu_operator_cost * outputRows; + } + else + { + /* + * In hashed mode, we must read all the input before we can emit + * anything. Also charge comparison costs to represent the cost of + * hash table lookups. + */ + pathnode->path.startup_cost = + leftpath->total_cost + rightpath->total_cost + + cpu_operator_cost * (leftpath->rows + rightpath->rows) * list_length(groupList); + pathnode->path.total_cost = pathnode->path.startup_cost; + + /* + * Also charge a small amount per extracted tuple. Like cost_sort, + * charge only operator cost not cpu_tuple_cost, since SetOp does no + * qual-checking or projection. + */ + pathnode->path.total_cost += cpu_operator_cost * outputRows; + } pathnode->path.rows = outputRows; return pathnode; diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index 154fffabae..6f2926c45a 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -1221,8 +1221,10 @@ where o.ten = 0; (1 row) -- --- Test rescan of a SetOp node +-- Test rescan of a hashed SetOp node -- +begin; +set local enable_sort = off; explain (costs off) select count(*) from onek o cross join lateral ( @@ -1256,6 +1258,50 @@ where o.ten = 1; 100 (1 row) +rollback; +-- +-- Test rescan of a sorted SetOp node +-- +begin; +set local enable_hashagg = off; +explain (costs off) +select count(*) from + onek o cross join lateral ( + select * from onek i1 where i1.unique1 = o.unique1 + except + select * from onek i2 where i2.unique1 = o.unique2 + ) ss +where o.ten = 1; + QUERY PLAN +--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + Aggregate + -> Nested Loop + -> Seq Scan on onek o + Filter: (ten = 1) + -> SetOp Except + -> Sort + Sort Key: i1.unique1, i1.unique2, i1.two, i1.four, i1.ten, i1.twenty, i1.hundred, i1.thousand, i1.twothousand, i1.fivethous, i1.tenthous, i1.odd, i1.even, i1.stringu1, i1.stringu2, i1.string4 + -> Index Scan using onek_unique1 on onek i1 + Index Cond: (unique1 = o.unique1) + -> Sort + Sort Key: i2.unique1, i2.unique2, i2.two, i2.four, i2.ten, i2.twenty, i2.hundred, i2.thousand, i2.twothousand, i2.fivethous, i2.tenthous, i2.odd, i2.even, i2.stringu1, i2.stringu2, i2.string4 + -> Index Scan using onek_unique1 on onek i2 + Index Cond: (unique1 = o.unique2) +(13 rows) + +select count(*) from + onek o cross join lateral ( + select * from onek i1 where i1.unique1 = o.unique1 + except + select * from onek i2 where i2.unique1 = o.unique2 + ) ss +where o.ten = 1; + count +------- + 100 +(1 row) + +rollback; -- -- Test rescan of a RecursiveUnion node -- diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out index fbf24490e9..9474b01510 100644 --- a/src/test/regress/expected/union.out +++ b/src/test/regress/expected/union.out @@ -950,7 +950,7 @@ explain (costs off) select from generate_series(1,5) intersect select from generate_series(1,3); QUERY PLAN ---------------------------------------------------------- - HashSetOp Intersect + SetOp Intersect -> Function Scan on generate_series -> Function Scan on generate_series generate_series_1 (3 rows) diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index af6e157aca..11becd885c 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -638,8 +638,11 @@ select sum(ss.tst::int) from where o.ten = 0; -- --- Test rescan of a SetOp node +-- Test rescan of a hashed SetOp node -- +begin; +set local enable_sort = off; + explain (costs off) select count(*) from onek o cross join lateral ( @@ -657,6 +660,33 @@ select count(*) from ) ss where o.ten = 1; +rollback; + +-- +-- Test rescan of a sorted SetOp node +-- +begin; +set local enable_hashagg = off; + +explain (costs off) +select count(*) from + onek o cross join lateral ( + select * from onek i1 where i1.unique1 = o.unique1 + except + select * from onek i2 where i2.unique1 = o.unique2 + ) ss +where o.ten = 1; + +select count(*) from + onek o cross join lateral ( + select * from onek i1 where i1.unique1 = o.unique1 + except + select * from onek i2 where i2.unique1 = o.unique2 + ) ss +where o.ten = 1; + +rollback; + -- -- Test rescan of a RecursiveUnion node -- -- 2.43.5