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

Commit 8df08c8

Browse files
committed
Reimplement planner's handling of MIN/MAX aggregate optimization (again).
Instead of playing cute games with pathkeys, just build a direct representation of the intended sub-select, and feed it through query_planner to get a Path for the index access. This is a bit slower than 9.1's previous method, since we'll duplicate most of the overhead of query_planner; but since the whole optimization only applies to rather simple single-table queries, that probably won't be much of a problem in practice. The advantage is that we get to do the right thing when there's a partial index that needs the implicit IS NOT NULL clause to be usable. Also, although this makes planagg.c be a bit more closely tied to the ordering of operations in grouping_planner, we can get rid of some coupling to lower-level parts of the planner. Per complaint from Marti Raudsepp.
1 parent 6d8096e commit 8df08c8

File tree

12 files changed

+263
-545
lines changed

12 files changed

+263
-545
lines changed

src/backend/nodes/copyfuncs.c

-19
Original file line numberDiff line numberDiff line change
@@ -1930,22 +1930,6 @@ _copyPlaceHolderInfo(PlaceHolderInfo *from)
19301930
return newnode;
19311931
}
19321932

1933-
/*
1934-
* _copyMinMaxAggInfo
1935-
*/
1936-
static MinMaxAggInfo *
1937-
_copyMinMaxAggInfo(MinMaxAggInfo *from)
1938-
{
1939-
MinMaxAggInfo *newnode = makeNode(MinMaxAggInfo);
1940-
1941-
COPY_SCALAR_FIELD(aggfnoid);
1942-
COPY_SCALAR_FIELD(aggsortop);
1943-
COPY_NODE_FIELD(target);
1944-
COPY_NODE_FIELD(pathkeys);
1945-
1946-
return newnode;
1947-
}
1948-
19491933
/* ****************************************************************
19501934
* parsenodes.h copy functions
19511935
* ****************************************************************
@@ -4129,9 +4113,6 @@ copyObject(void *from)
41294113
case T_PlaceHolderInfo:
41304114
retval = _copyPlaceHolderInfo(from);
41314115
break;
4132-
case T_MinMaxAggInfo:
4133-
retval = _copyMinMaxAggInfo(from);
4134-
break;
41354116

41364117
/*
41374118
* VALUE NODES

src/backend/nodes/equalfuncs.c

-14
Original file line numberDiff line numberDiff line change
@@ -886,17 +886,6 @@ _equalPlaceHolderInfo(PlaceHolderInfo *a, PlaceHolderInfo *b)
886886
return true;
887887
}
888888

889-
static bool
890-
_equalMinMaxAggInfo(MinMaxAggInfo *a, MinMaxAggInfo *b)
891-
{
892-
COMPARE_SCALAR_FIELD(aggfnoid);
893-
COMPARE_SCALAR_FIELD(aggsortop);
894-
COMPARE_NODE_FIELD(target);
895-
COMPARE_NODE_FIELD(pathkeys);
896-
897-
return true;
898-
}
899-
900889

901890
/*
902891
* Stuff from parsenodes.h
@@ -2690,9 +2679,6 @@ equal(void *a, void *b)
26902679
case T_PlaceHolderInfo:
26912680
retval = _equalPlaceHolderInfo(a, b);
26922681
break;
2693-
case T_MinMaxAggInfo:
2694-
retval = _equalMinMaxAggInfo(a, b);
2695-
break;
26962682

26972683
case T_List:
26982684
case T_IntList:

src/backend/nodes/outfuncs.c

+4-1
Original file line numberDiff line numberDiff line change
@@ -1914,7 +1914,10 @@ _outMinMaxAggInfo(StringInfo str, MinMaxAggInfo *node)
19141914
WRITE_OID_FIELD(aggfnoid);
19151915
WRITE_OID_FIELD(aggsortop);
19161916
WRITE_NODE_FIELD(target);
1917-
WRITE_NODE_FIELD(pathkeys);
1917+
/* We intentionally omit subroot --- too large, not interesting enough */
1918+
WRITE_NODE_FIELD(path);
1919+
WRITE_FLOAT_FIELD(pathcost, "%.2f");
1920+
WRITE_NODE_FIELD(param);
19181921
}
19191922

19201923
static void

src/backend/optimizer/path/indxpath.c

+13-1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,13 @@
4141
#define IsBooleanOpfamily(opfamily) \
4242
((opfamily) == BOOL_BTREE_FAM_OID || (opfamily) == BOOL_HASH_FAM_OID)
4343

44+
/* Whether to use ScalarArrayOpExpr to build index qualifications */
45+
typedef enum
46+
{
47+
SAOP_FORBID, /* Do not use ScalarArrayOpExpr */
48+
SAOP_ALLOW, /* OK to use ScalarArrayOpExpr */
49+
SAOP_REQUIRE /* Require ScalarArrayOpExpr */
50+
} SaOpControl;
4451

4552
/* Whether we are looking for plain indexscan, bitmap scan, or either */
4653
typedef enum
@@ -78,6 +85,11 @@ static PathClauseUsage *classify_index_clause_usage(Path *path,
7885
List **clauselist);
7986
static void find_indexpath_quals(Path *bitmapqual, List **quals, List **preds);
8087
static int find_list_position(Node *node, List **nodelist);
88+
static List *group_clauses_by_indexkey(IndexOptInfo *index,
89+
List *clauses, List *outer_clauses,
90+
Relids outer_relids,
91+
SaOpControl saop_control,
92+
bool *found_clause);
8193
static bool match_clause_to_indexcol(IndexOptInfo *index,
8294
int indexcol,
8395
RestrictInfo *rinfo,
@@ -1060,7 +1072,7 @@ find_list_position(Node *node, List **nodelist)
10601072
* from multiple places. Defend against redundant outputs by using
10611073
* list_append_unique_ptr (pointer equality should be good enough).
10621074
*/
1063-
List *
1075+
static List *
10641076
group_clauses_by_indexkey(IndexOptInfo *index,
10651077
List *clauses, List *outer_clauses,
10661078
Relids outer_relids,

src/backend/optimizer/path/pathkeys.c

+6-90
Original file line numberDiff line numberDiff line change
@@ -905,39 +905,6 @@ make_pathkeys_for_sortclauses(PlannerInfo *root,
905905
return pathkeys;
906906
}
907907

908-
/****************************************************************************
909-
* PATHKEYS AND AGGREGATES
910-
****************************************************************************/
911-
912-
/*
913-
* make_pathkeys_for_aggregate
914-
* Generate a pathkeys list (always a 1-item list) that represents
915-
* the sort order needed by a MIN/MAX aggregate
916-
*
917-
* This is only called before EquivalenceClass merging, so we can assume
918-
* we are not supposed to canonicalize.
919-
*/
920-
List *
921-
make_pathkeys_for_aggregate(PlannerInfo *root,
922-
Expr *aggtarget,
923-
Oid aggsortop)
924-
{
925-
PathKey *pathkey;
926-
927-
/*
928-
* We arbitrarily set nulls_first to false. Actually, a MIN/MAX agg can
929-
* use either nulls ordering option, but that is dealt with elsewhere.
930-
*/
931-
pathkey = make_pathkey_from_sortop(root,
932-
aggtarget,
933-
aggsortop,
934-
false, /* nulls_first */
935-
0,
936-
true,
937-
false);
938-
return list_make1(pathkey);
939-
}
940-
941908
/****************************************************************************
942909
* PATHKEYS AND MERGECLAUSES
943910
****************************************************************************/
@@ -1407,11 +1374,10 @@ make_inner_pathkeys_for_merge(PlannerInfo *root,
14071374
* PATHKEY USEFULNESS CHECKS
14081375
*
14091376
* We only want to remember as many of the pathkeys of a path as have some
1410-
* potential use, which can include subsequent mergejoins, meeting the query's
1411-
* requested output ordering, or implementing MIN/MAX aggregates. This
1412-
* ensures that add_path() won't consider a path to have a usefully different
1413-
* ordering unless it really is useful. These routines check for usefulness
1414-
* of given pathkeys.
1377+
* potential use, either for subsequent mergejoins or for meeting the query's
1378+
* requested output ordering. This ensures that add_path() won't consider
1379+
* a path to have a usefully different ordering unless it really is useful.
1380+
* These routines check for usefulness of given pathkeys.
14151381
****************************************************************************/
14161382

14171383
/*
@@ -1553,50 +1519,6 @@ pathkeys_useful_for_ordering(PlannerInfo *root, List *pathkeys)
15531519
return 0; /* path ordering not useful */
15541520
}
15551521

1556-
/*
1557-
* pathkeys_useful_for_minmax
1558-
* Count the number of pathkeys that are useful for implementing
1559-
* some MIN/MAX aggregate.
1560-
*
1561-
* Like pathkeys_useful_for_ordering, this is a yes-or-no affair, but
1562-
* there could be several MIN/MAX aggregates and we can match to any one.
1563-
*
1564-
* We can't use pathkeys_contained_in() because we would like to match
1565-
* pathkeys regardless of the nulls_first setting. However, we know that
1566-
* MIN/MAX aggregates will have at most one item in their pathkeys, so it's
1567-
* not too complicated to match by brute force.
1568-
*/
1569-
static int
1570-
pathkeys_useful_for_minmax(PlannerInfo *root, List *pathkeys)
1571-
{
1572-
PathKey *pathkey;
1573-
ListCell *lc;
1574-
1575-
if (pathkeys == NIL)
1576-
return 0; /* unordered path */
1577-
pathkey = (PathKey *) linitial(pathkeys);
1578-
1579-
foreach(lc, root->minmax_aggs)
1580-
{
1581-
MinMaxAggInfo *mminfo = (MinMaxAggInfo *) lfirst(lc);
1582-
PathKey *mmpathkey;
1583-
1584-
/* Ignore minmax agg if its pathkey turned out to be redundant */
1585-
if (mminfo->pathkeys == NIL)
1586-
continue;
1587-
1588-
Assert(list_length(mminfo->pathkeys) == 1);
1589-
mmpathkey = (PathKey *) linitial(mminfo->pathkeys);
1590-
1591-
if (mmpathkey->pk_eclass == pathkey->pk_eclass &&
1592-
mmpathkey->pk_opfamily == pathkey->pk_opfamily &&
1593-
mmpathkey->pk_strategy == pathkey->pk_strategy)
1594-
return 1;
1595-
}
1596-
1597-
return 0; /* path ordering not useful */
1598-
}
1599-
16001522
/*
16011523
* truncate_useless_pathkeys
16021524
* Shorten the given pathkey list to just the useful pathkeys.
@@ -1608,15 +1530,11 @@ truncate_useless_pathkeys(PlannerInfo *root,
16081530
{
16091531
int nuseful;
16101532
int nuseful2;
1611-
int nuseful3;
16121533

16131534
nuseful = pathkeys_useful_for_merging(root, rel, pathkeys);
16141535
nuseful2 = pathkeys_useful_for_ordering(root, pathkeys);
16151536
if (nuseful2 > nuseful)
16161537
nuseful = nuseful2;
1617-
nuseful3 = pathkeys_useful_for_minmax(root, pathkeys);
1618-
if (nuseful3 > nuseful)
1619-
nuseful = nuseful3;
16201538

16211539
/*
16221540
* Note: not safe to modify input list destructively, but we can avoid
@@ -1642,8 +1560,8 @@ truncate_useless_pathkeys(PlannerInfo *root,
16421560
*
16431561
* We could make the test more complex, for example checking to see if any of
16441562
* the joinclauses are really mergejoinable, but that likely wouldn't win
1645-
* often enough to repay the extra cycles. Queries with no join, sort, or
1646-
* aggregate at all are reasonably common, so this much work seems worthwhile.
1563+
* often enough to repay the extra cycles. Queries with neither a join nor
1564+
* a sort are reasonably common, though, so this much work seems worthwhile.
16471565
*/
16481566
bool
16491567
has_useful_pathkeys(PlannerInfo *root, RelOptInfo *rel)
@@ -1652,7 +1570,5 @@ has_useful_pathkeys(PlannerInfo *root, RelOptInfo *rel)
16521570
return true; /* might be able to use pathkeys for merging */
16531571
if (root->query_pathkeys != NIL)
16541572
return true; /* might be able to use them for ordering */
1655-
if (root->minmax_aggs != NIL)
1656-
return true; /* might be able to use them for MIN/MAX */
16571573
return false; /* definitely useless */
16581574
}

0 commit comments

Comments
 (0)