Re: Performance regression: 9.2+ vs. ScalarArrayOpExpr vs. ORDER BY
От | Bruce Momjian |
---|---|
Тема | Re: Performance regression: 9.2+ vs. ScalarArrayOpExpr vs. ORDER BY |
Дата | |
Msg-id | 20141011204029.GG21267@momjian.us обсуждение исходный текст |
Ответ на | Re: Performance regression: 9.2+ vs. ScalarArrayOpExpr vs. ORDER BY (Andrew Gierth <andrew@tao11.riddles.org.uk>) |
Ответы |
Re: Performance regression: 9.2+ vs. ScalarArrayOpExpr
vs. ORDER BY
|
Список | pgsql-hackers |
Uh, did this ever get addressed? --------------------------------------------------------------------------- On Sun, Jul 6, 2014 at 08:56:00PM +0100, Andrew Gierth wrote: > >>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes: > > >> I've experimented with the attached patch, which detects when this > >> situation might have occurred and does another pass to try and > >> build ordered scans without the SAOP condition. However, the > >> results may not be quite ideal, because at least in some test > >> queries (not all) the scan with the SAOP included in the > >> indexquals is being costed higher than the same scan with the SAOP > >> moved to a Filter, which seems unreasonable. > > Tom> I'm not convinced that that's a-priori unreasonable, since the > Tom> SAOP will result in multiple index scans under the hood. > Tom> Conceivably we ought to try the path with and with SAOPs all the > Tom> time. > > OK, here's a patch that always retries on lower SAOP clauses, assuming > that a SAOP in the first column is always applicable - or is even that > assumption too strong? > > -- > Andrew (irc:RhodiumToad) > > diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c > index 42dcb11..cfcfbfc 100644 > --- a/src/backend/optimizer/path/indxpath.c > +++ b/src/backend/optimizer/path/indxpath.c > @@ -50,7 +50,8 @@ typedef enum > { > SAOP_PER_AM, /* Use ScalarArrayOpExpr if amsearcharray */ > SAOP_ALLOW, /* Use ScalarArrayOpExpr for all indexes */ > - SAOP_REQUIRE /* Require ScalarArrayOpExpr to be used */ > + SAOP_REQUIRE, /* Require ScalarArrayOpExpr to be used */ > + SAOP_SKIP_LOWER /* Require lower ScalarArrayOpExpr to be eliminated */ > } SaOpControl; > > /* Whether we are looking for plain indexscan, bitmap scan, or either */ > @@ -118,7 +119,8 @@ static void get_index_paths(PlannerInfo *root, RelOptInfo *rel, > static List *build_index_paths(PlannerInfo *root, RelOptInfo *rel, > IndexOptInfo *index, IndexClauseSet *clauses, > bool useful_predicate, > - SaOpControl saop_control, ScanTypeControl scantype); > + SaOpControl saop_control, ScanTypeControl scantype, > + bool *saop_retry); > static List *build_paths_for_OR(PlannerInfo *root, RelOptInfo *rel, > List *clauses, List *other_clauses); > static List *generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel, > @@ -734,6 +736,7 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel, > { > List *indexpaths; > ListCell *lc; > + bool saop_retry = false; > > /* > * Build simple index paths using the clauses. Allow ScalarArrayOpExpr > @@ -742,7 +745,23 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel, > indexpaths = build_index_paths(root, rel, > index, clauses, > index->predOK, > - SAOP_PER_AM, ST_ANYSCAN); > + SAOP_PER_AM, ST_ANYSCAN, &saop_retry); > + > + /* > + * If we allowed any ScalarArrayOpExprs on an index with a useful sort > + * ordering, then try again without them, since otherwise we miss important > + * paths where the index ordering is relevant. > + */ > + if (saop_retry) > + { > + indexpaths = list_concat(indexpaths, > + build_index_paths(root, rel, > + index, clauses, > + index->predOK, > + SAOP_SKIP_LOWER, > + ST_ANYSCAN, > + NULL)); > + } > > /* > * Submit all the ones that can form plain IndexScan plans to add_path. (A > @@ -779,7 +798,7 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel, > indexpaths = build_index_paths(root, rel, > index, clauses, > false, > - SAOP_REQUIRE, ST_BITMAPSCAN); > + SAOP_REQUIRE, ST_BITMAPSCAN, NULL); > *bitindexpaths = list_concat(*bitindexpaths, indexpaths); > } > } > @@ -816,12 +835,14 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel, > * 'useful_predicate' indicates whether the index has a useful predicate > * 'saop_control' indicates whether ScalarArrayOpExpr clauses can be used > * 'scantype' indicates whether we need plain or bitmap scan support > + * 'saop_retry' indicates whether a SAOP_SKIP_LOWER retry is worthwhile > */ > static List * > build_index_paths(PlannerInfo *root, RelOptInfo *rel, > IndexOptInfo *index, IndexClauseSet *clauses, > bool useful_predicate, > - SaOpControl saop_control, ScanTypeControl scantype) > + SaOpControl saop_control, ScanTypeControl scantype, > + bool *saop_retry) > { > List *result = NIL; > IndexPath *ipath; > @@ -877,7 +898,9 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel, > * assuming that the scan result is ordered. (Actually, the result is > * still ordered if there are equality constraints for all earlier > * columns, but it seems too expensive and non-modular for this code to be > - * aware of that refinement.) > + * aware of that refinement.) But if saop_control is SAOP_SKIP_LOWER, we > + * skip exactly these clauses that break sorting, and don't bother > + * building any paths otherwise. > * > * We also build a Relids set showing which outer rels are required by the > * selected clauses. Any lateral_relids are included in that, but not > @@ -901,9 +924,13 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel, > /* Ignore if not supported by index */ > if (saop_control == SAOP_PER_AM && !index->amsearcharray) > continue; > - found_clause = true; > if (indexcol > 0) > + { > found_lower_saop_clause = true; > + if (saop_control == SAOP_SKIP_LOWER) > + continue; > + } > + found_clause = true; > } > else > { > @@ -928,6 +955,29 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel, > return NIL; > } > > + if (saop_control == SAOP_SKIP_LOWER) > + { > + if (!found_lower_saop_clause) > + return NIL; > + found_lower_saop_clause = false; > + } > + else if (found_lower_saop_clause) > + { > + /* > + * If we have a lower SAOP clause, we should leave it up to the cost > + * estimates to decide whether to use it in the scan or punt it to a > + * filter clause, rather than try and second-guess the AM's cost > + * estimate mechanism. In addition, we need to consider the path > + * without the SAOP on the basis that it might give us an ordering > + * which overcomes any cost advantage of using the SAOP as an index > + * qual. So either way, we flag it up so that the caller can do > + * another pass over the same index with SAOP_SKIP_LOWER to catch > + * such cases. > + */ > + if (saop_retry) > + *saop_retry = true; > + } > + > /* We do not want the index's rel itself listed in outer_relids */ > outer_relids = bms_del_member(outer_relids, rel->relid); > /* Enforce convention that outer_relids is exactly NULL if empty */ > @@ -1137,7 +1187,7 @@ build_paths_for_OR(PlannerInfo *root, RelOptInfo *rel, > indexpaths = build_index_paths(root, rel, > index, &clauseset, > useful_predicate, > - SAOP_ALLOW, ST_BITMAPSCAN); > + SAOP_ALLOW, ST_BITMAPSCAN, NULL); > result = list_concat(result, indexpaths); > } > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Bruce MomjianДата:
Сообщение: Re: No toast table for pg_shseclabel but for pg_seclabel