Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 7204f35

Browse files
committed
Revert commit 66c0185 and follow-on patches.
This reverts 66c0185 (Allow planner to use Merge Append to efficiently implement UNION) as well as the follow-on commits d5d2205, 3b1a7eb, 7487044. In addition to those, 07746a8 had to be removed then re-applied in a different place, because 66c0185 moved the relevant code. The reason for this last-minute thrashing is that depesz found a case in which the patched code creates a completely wrong plan that silently gives incorrect query results. It's unclear what the cause is or how many cases are affected, but with beta1 wrap staring us in the face, there's no time for closer investigation. After we figure that out, we can decide whether to un-revert this for beta2 or hold it for v18. Discussion: https://postgr.es/m/Zktzf926vslR35Fv@depesz.com (also some private discussion among pgsql-release)
1 parent d2a0447 commit 7204f35

File tree

18 files changed

+287
-761
lines changed

18 files changed

+287
-761
lines changed

contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11511,10 +11511,6 @@ DROP INDEX base_tbl1_idx;
1151111511
DROP INDEX base_tbl2_idx;
1151211512
DROP INDEX async_p3_idx;
1151311513
-- UNION queries
11514-
SET enable_sort TO off;
11515-
SET enable_incremental_sort TO off;
11516-
-- Adjust fdw_startup_cost so that we get an unordered path in the Append.
11517-
ALTER SERVER loopback2 OPTIONS (ADD fdw_startup_cost '0.00');
1151811514
EXPLAIN (VERBOSE, COSTS OFF)
1151911515
INSERT INTO result_tbl
1152011516
(SELECT a, b, 'AAA' || c FROM async_p1 ORDER BY a LIMIT 10)
@@ -11596,9 +11592,6 @@ SELECT * FROM result_tbl ORDER BY a;
1159611592
(12 rows)
1159711593

1159811594
DELETE FROM result_tbl;
11599-
RESET enable_incremental_sort;
11600-
RESET enable_sort;
11601-
ALTER SERVER loopback2 OPTIONS (DROP fdw_startup_cost);
1160211595
-- Disable async execution if we use gating Result nodes for pseudoconstant
1160311596
-- quals
1160411597
EXPLAIN (VERBOSE, COSTS OFF)

contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3885,11 +3885,6 @@ DROP INDEX base_tbl2_idx;
38853885
DROP INDEX async_p3_idx;
38863886

38873887
-- UNION queries
3888-
SET enable_sort TO off;
3889-
SET enable_incremental_sort TO off;
3890-
-- Adjust fdw_startup_cost so that we get an unordered path in the Append.
3891-
ALTER SERVER loopback2 OPTIONS (ADD fdw_startup_cost '0.00');
3892-
38933888
EXPLAIN (VERBOSE, COSTS OFF)
38943889
INSERT INTO result_tbl
38953890
(SELECT a, b, 'AAA' || c FROM async_p1 ORDER BY a LIMIT 10)
@@ -3916,10 +3911,6 @@ UNION ALL
39163911
SELECT * FROM result_tbl ORDER BY a;
39173912
DELETE FROM result_tbl;
39183913

3919-
RESET enable_incremental_sort;
3920-
RESET enable_sort;
3921-
ALTER SERVER loopback2 OPTIONS (DROP fdw_startup_cost);
3922-
39233914
-- Disable async execution if we use gating Result nodes for pseudoconstant
39243915
-- quals
39253916
EXPLAIN (VERBOSE, COSTS OFF)

src/backend/optimizer/path/allpaths.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2633,8 +2633,9 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
26332633
Assert(root->plan_params == NIL);
26342634

26352635
/* Generate a subroot and Paths for the subquery */
2636-
rel->subroot = subquery_planner(root->glob, subquery, root, false,
2637-
tuple_fraction, NULL);
2636+
rel->subroot = subquery_planner(root->glob, subquery,
2637+
root,
2638+
false, tuple_fraction);
26382639

26392640
/* Isolate the params needed by this specific subplan */
26402641
rel->subplan_params = root->plan_params;

src/backend/optimizer/path/equivclass.c

Lines changed: 0 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -2882,67 +2882,6 @@ add_child_join_rel_equivalences(PlannerInfo *root,
28822882
MemoryContextSwitchTo(oldcontext);
28832883
}
28842884

2885-
/*
2886-
* add_setop_child_rel_equivalences
2887-
* Add equivalence members for each non-resjunk target in 'child_tlist'
2888-
* to the EquivalenceClass in the corresponding setop_pathkey's pk_eclass.
2889-
*
2890-
* 'root' is the PlannerInfo belonging to the top-level set operation.
2891-
* 'child_rel' is the RelOptInfo of the child relation we're adding
2892-
* EquivalenceMembers for.
2893-
* 'child_tlist' is the target list for the setop child relation. The target
2894-
* list expressions are what we add as EquivalenceMembers.
2895-
* 'setop_pathkeys' is a list of PathKeys which must contain an entry for each
2896-
* non-resjunk target in 'child_tlist'.
2897-
*/
2898-
void
2899-
add_setop_child_rel_equivalences(PlannerInfo *root, RelOptInfo *child_rel,
2900-
List *child_tlist, List *setop_pathkeys)
2901-
{
2902-
ListCell *lc;
2903-
ListCell *lc2 = list_head(setop_pathkeys);
2904-
2905-
foreach(lc, child_tlist)
2906-
{
2907-
TargetEntry *tle = lfirst_node(TargetEntry, lc);
2908-
EquivalenceMember *parent_em;
2909-
PathKey *pk;
2910-
2911-
if (tle->resjunk)
2912-
continue;
2913-
2914-
if (lc2 == NULL)
2915-
elog(ERROR, "too few pathkeys for set operation");
2916-
2917-
pk = lfirst_node(PathKey, lc2);
2918-
parent_em = linitial(pk->pk_eclass->ec_members);
2919-
2920-
/*
2921-
* We can safely pass the parent member as the first member in the
2922-
* ec_members list as this is added first in generate_union_paths,
2923-
* likewise, the JoinDomain can be that of the initial member of the
2924-
* Pathkey's EquivalenceClass.
2925-
*/
2926-
add_eq_member(pk->pk_eclass,
2927-
tle->expr,
2928-
child_rel->relids,
2929-
parent_em->em_jdomain,
2930-
parent_em,
2931-
exprType((Node *) tle->expr));
2932-
2933-
lc2 = lnext(setop_pathkeys, lc2);
2934-
}
2935-
2936-
/*
2937-
* transformSetOperationStmt() ensures that the targetlist never contains
2938-
* any resjunk columns, so all eclasses that exist in 'root' must have
2939-
* received a new member in the loop above. Add them to the child_rel's
2940-
* eclass_indexes.
2941-
*/
2942-
child_rel->eclass_indexes = bms_add_range(child_rel->eclass_indexes, 0,
2943-
list_length(root->eq_classes) - 1);
2944-
}
2945-
29462885

29472886
/*
29482887
* generate_implied_equalities_for_column

src/backend/optimizer/path/pathkeys.c

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2191,22 +2191,6 @@ pathkeys_useful_for_grouping(PlannerInfo *root, List *pathkeys)
21912191
return n;
21922192
}
21932193

2194-
/*
2195-
* pathkeys_useful_for_setop
2196-
* Count the number of leading common pathkeys root's 'setop_pathkeys' in
2197-
* 'pathkeys'.
2198-
*/
2199-
static int
2200-
pathkeys_useful_for_setop(PlannerInfo *root, List *pathkeys)
2201-
{
2202-
int n_common_pathkeys;
2203-
2204-
(void) pathkeys_count_contained_in(root->setop_pathkeys, pathkeys,
2205-
&n_common_pathkeys);
2206-
2207-
return n_common_pathkeys;
2208-
}
2209-
22102194
/*
22112195
* truncate_useless_pathkeys
22122196
* Shorten the given pathkey list to just the useful pathkeys.
@@ -2224,9 +2208,6 @@ truncate_useless_pathkeys(PlannerInfo *root,
22242208
if (nuseful2 > nuseful)
22252209
nuseful = nuseful2;
22262210
nuseful2 = pathkeys_useful_for_grouping(root, pathkeys);
2227-
if (nuseful2 > nuseful)
2228-
nuseful = nuseful2;
2229-
nuseful2 = pathkeys_useful_for_setop(root, pathkeys);
22302211
if (nuseful2 > nuseful)
22312212
nuseful = nuseful2;
22322213

src/backend/optimizer/plan/planner.c

Lines changed: 21 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@
5454
#include "optimizer/tlist.h"
5555
#include "parser/analyze.h"
5656
#include "parser/parse_agg.h"
57-
#include "parser/parse_clause.h"
5857
#include "parser/parse_relation.h"
5958
#include "parser/parsetree.h"
6059
#include "partitioning/partdesc.h"
@@ -120,15 +119,12 @@ typedef struct
120119
{
121120
List *activeWindows; /* active windows, if any */
122121
grouping_sets_data *gset_data; /* grouping sets data, if any */
123-
SetOperationStmt *setop; /* parent set operation or NULL if not a
124-
* subquery belonging to a set operation */
125122
} standard_qp_extra;
126123

127124
/* Local functions */
128125
static Node *preprocess_expression(PlannerInfo *root, Node *expr, int kind);
129126
static void preprocess_qual_conditions(PlannerInfo *root, Node *jtnode);
130-
static void grouping_planner(PlannerInfo *root, double tuple_fraction,
131-
SetOperationStmt *setops);
127+
static void grouping_planner(PlannerInfo *root, double tuple_fraction);
132128
static grouping_sets_data *preprocess_grouping_sets(PlannerInfo *root);
133129
static List *remap_to_groupclause_idx(List *groupClause, List *gsets,
134130
int *tleref_to_colnum_map);
@@ -253,8 +249,6 @@ static bool group_by_has_partkey(RelOptInfo *input_rel,
253249
List *targetList,
254250
List *groupClause);
255251
static int common_prefix_cmp(const void *a, const void *b);
256-
static List *generate_setop_child_grouplist(SetOperationStmt *op,
257-
List *targetlist);
258252

259253

260254
/*****************************************************************************
@@ -412,7 +406,8 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
412406
}
413407

414408
/* primary planning entry point (may recurse for subqueries) */
415-
root = subquery_planner(glob, parse, NULL, false, tuple_fraction, NULL);
409+
root = subquery_planner(glob, parse, NULL,
410+
false, tuple_fraction);
416411

417412
/* Select best Path and turn it into a Plan */
418413
final_rel = fetch_upper_rel(root, UPPERREL_FINAL, NULL);
@@ -603,10 +598,6 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
603598
* hasRecursion is true if this is a recursive WITH query.
604599
* tuple_fraction is the fraction of tuples we expect will be retrieved.
605600
* tuple_fraction is interpreted as explained for grouping_planner, below.
606-
* setops is used for set operation subqueries to provide the subquery with
607-
* the context in which it's being used so that Paths correctly sorted for the
608-
* set operation can be generated. NULL when not planning a set operation
609-
* child.
610601
*
611602
* Basically, this routine does the stuff that should only be done once
612603
* per Query object. It then calls grouping_planner. At one time,
@@ -625,9 +616,9 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
625616
*--------------------
626617
*/
627618
PlannerInfo *
628-
subquery_planner(PlannerGlobal *glob, Query *parse, PlannerInfo *parent_root,
629-
bool hasRecursion, double tuple_fraction,
630-
SetOperationStmt *setops)
619+
subquery_planner(PlannerGlobal *glob, Query *parse,
620+
PlannerInfo *parent_root,
621+
bool hasRecursion, double tuple_fraction)
631622
{
632623
PlannerInfo *root;
633624
List *newWithCheckOptions;
@@ -1086,7 +1077,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse, PlannerInfo *parent_root,
10861077
/*
10871078
* Do the main planning.
10881079
*/
1089-
grouping_planner(root, tuple_fraction, setops);
1080+
grouping_planner(root, tuple_fraction);
10901081

10911082
/*
10921083
* Capture the set of outer-level param IDs we have access to, for use in
@@ -1284,11 +1275,7 @@ preprocess_phv_expression(PlannerInfo *root, Expr *expr)
12841275
* 0 < tuple_fraction < 1: expect the given fraction of tuples available
12851276
* from the plan to be retrieved
12861277
* tuple_fraction >= 1: tuple_fraction is the absolute number of tuples
1287-
* expected to be retrieved (ie, a LIMIT specification).
1288-
* setops is used for set operation subqueries to provide the subquery with
1289-
* the context in which it's being used so that Paths correctly sorted for the
1290-
* set operation can be generated. NULL when not planning a set operation
1291-
* child.
1278+
* expected to be retrieved (ie, a LIMIT specification)
12921279
*
12931280
* Returns nothing; the useful output is in the Paths we attach to the
12941281
* (UPPERREL_FINAL, NULL) upperrel in *root. In addition,
@@ -1299,8 +1286,7 @@ preprocess_phv_expression(PlannerInfo *root, Expr *expr)
12991286
*--------------------
13001287
*/
13011288
static void
1302-
grouping_planner(PlannerInfo *root, double tuple_fraction,
1303-
SetOperationStmt *setops)
1289+
grouping_planner(PlannerInfo *root, double tuple_fraction)
13041290
{
13051291
Query *parse = root->parse;
13061292
int64 offset_est = 0;
@@ -1335,6 +1321,17 @@ grouping_planner(PlannerInfo *root, double tuple_fraction,
13351321

13361322
if (parse->setOperations)
13371323
{
1324+
/*
1325+
* If there's a top-level ORDER BY, assume we have to fetch all the
1326+
* tuples. This might be too simplistic given all the hackery below
1327+
* to possibly avoid the sort; but the odds of accurate estimates here
1328+
* are pretty low anyway. XXX try to get rid of this in favor of
1329+
* letting plan_set_operations generate both fast-start and
1330+
* cheapest-total paths.
1331+
*/
1332+
if (parse->sortClause)
1333+
root->tuple_fraction = 0.0;
1334+
13381335
/*
13391336
* Construct Paths for set operations. The results will not need any
13401337
* work except perhaps a top-level sort and/or LIMIT. Note that any
@@ -1504,12 +1501,6 @@ grouping_planner(PlannerInfo *root, double tuple_fraction,
15041501
qp_extra.activeWindows = activeWindows;
15051502
qp_extra.gset_data = gset_data;
15061503

1507-
/*
1508-
* If we're a subquery for a set operation, store the SetOperationStmt
1509-
* in qp_extra.
1510-
*/
1511-
qp_extra.setop = setops;
1512-
15131504
/*
15141505
* Generate the best unsorted and presorted paths for the scan/join
15151506
* portion of this Query, ie the processing represented by the
@@ -3462,27 +3453,6 @@ standard_qp_callback(PlannerInfo *root, void *extra)
34623453
parse->sortClause,
34633454
tlist);
34643455

3465-
/* setting setop_pathkeys might be useful to the union planner */
3466-
if (qp_extra->setop != NULL &&
3467-
set_operation_ordered_results_useful(qp_extra->setop))
3468-
{
3469-
List *groupClauses;
3470-
bool sortable;
3471-
3472-
groupClauses = generate_setop_child_grouplist(qp_extra->setop, tlist);
3473-
3474-
root->setop_pathkeys =
3475-
make_pathkeys_for_sortclauses_extended(root,
3476-
&groupClauses,
3477-
tlist,
3478-
false,
3479-
&sortable);
3480-
if (!sortable)
3481-
root->setop_pathkeys = NIL;
3482-
}
3483-
else
3484-
root->setop_pathkeys = NIL;
3485-
34863456
/*
34873457
* Figure out whether we want a sorted result from query_planner.
34883458
*
@@ -3492,9 +3462,7 @@ standard_qp_callback(PlannerInfo *root, void *extra)
34923462
* sortable DISTINCT clause that's more rigorous than the ORDER BY clause,
34933463
* we try to produce output that's sufficiently well sorted for the
34943464
* DISTINCT. Otherwise, if there is an ORDER BY clause, we want to sort
3495-
* by the ORDER BY clause. Otherwise, if we're a subquery being planned
3496-
* for a set operation which can benefit from presorted results and have a
3497-
* sortable targetlist, we want to sort by the target list.
3465+
* by the ORDER BY clause.
34983466
*
34993467
* Note: if we have both ORDER BY and GROUP BY, and ORDER BY is a superset
35003468
* of GROUP BY, it would be tempting to request sort by ORDER BY --- but
@@ -3512,8 +3480,6 @@ standard_qp_callback(PlannerInfo *root, void *extra)
35123480
root->query_pathkeys = root->distinct_pathkeys;
35133481
else if (root->sort_pathkeys)
35143482
root->query_pathkeys = root->sort_pathkeys;
3515-
else if (root->setop_pathkeys != NIL)
3516-
root->query_pathkeys = root->setop_pathkeys;
35173483
else
35183484
root->query_pathkeys = NIL;
35193485
}
@@ -7957,43 +7923,3 @@ group_by_has_partkey(RelOptInfo *input_rel,
79577923

79587924
return true;
79597925
}
7960-
7961-
/*
7962-
* generate_setop_child_grouplist
7963-
* Build a SortGroupClause list defining the sort/grouping properties
7964-
* of the child of a set operation.
7965-
*
7966-
* This is similar to generate_setop_grouplist() but differs as the setop
7967-
* child query's targetlist entries may already have a tleSortGroupRef
7968-
* assigned for other purposes, such as GROUP BYs. Here we keep the
7969-
* SortGroupClause list in the same order as 'op' groupClauses and just adjust
7970-
* the tleSortGroupRef to reference the TargetEntry's 'ressortgroupref'.
7971-
*/
7972-
static List *
7973-
generate_setop_child_grouplist(SetOperationStmt *op, List *targetlist)
7974-
{
7975-
List *grouplist = copyObject(op->groupClauses);
7976-
ListCell *lg;
7977-
ListCell *lt;
7978-
7979-
lg = list_head(grouplist);
7980-
foreach(lt, targetlist)
7981-
{
7982-
TargetEntry *tle = (TargetEntry *) lfirst(lt);
7983-
SortGroupClause *sgc;
7984-
7985-
/* resjunk columns could have sortgrouprefs. Leave these alone */
7986-
if (tle->resjunk)
7987-
continue;
7988-
7989-
/* we expect every non-resjunk target to have a SortGroupClause */
7990-
Assert(lg != NULL);
7991-
sgc = (SortGroupClause *) lfirst(lg);
7992-
lg = lnext(grouplist, lg);
7993-
7994-
/* assign a tleSortGroupRef, or reuse the existing one */
7995-
sgc->tleSortGroupRef = assignSortGroupRef(tle, targetlist);
7996-
}
7997-
Assert(lg == NULL);
7998-
return grouplist;
7999-
}

0 commit comments

Comments
 (0)