public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 8/8] Consider selectivity in choose_bitmap_and 4+ messages / 4 participants [nested] [flat]
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Tomas Vondra @ 2021-03-11 11:57 UTC (permalink / raw) --- src/backend/optimizer/path/indxpath.c | 38 ++++++++++++++++----------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index ff536e6b24..36b29c6c7b 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1437,6 +1437,7 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel, List *paths) foreach(l, paths) { Path *ipath = (Path *) lfirst(l); + bool duplicate = false; pathinfo = classify_index_clause_usage(ipath, &clauselist); @@ -1451,22 +1452,29 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel, List *paths) { if (!pathinfoarray[i]->unclassifiable && bms_equal(pathinfo->clauseids, pathinfoarray[i]->clauseids)) - break; - } - if (i < npaths) - { - /* duplicate clauseids, keep the cheaper one */ - Cost ncost; - Cost ocost; - Selectivity nselec; - Selectivity oselec; - - cost_bitmap_tree_node(pathinfo->path, &ncost, &nselec); - cost_bitmap_tree_node(pathinfoarray[i]->path, &ocost, &oselec); - if (ncost < ocost) - pathinfoarray[i] = pathinfo; + { + /* duplicate clauseids, keep the cheaper one + * + * XXX maybe this should just use path_usage_comparator? + */ + Cost ncost; + Cost ocost; + Selectivity nselec; + Selectivity oselec; + + cost_bitmap_tree_node(pathinfo->path, &ncost, &nselec); + cost_bitmap_tree_node(pathinfoarray[i]->path, &ocost, &oselec); + + if ((ncost < ocost) && (nselec < oselec)) + { + pathinfoarray[i] = pathinfo; + duplicate = true; + break; + } + } } - else + + if (!duplicate) { /* not duplicate clauseids, add to array */ pathinfoarray[npaths++] = pathinfo; -- 2.26.2 --------------D524CD37E77B36A2883A7617 Content-Type: application/sql; name="reproduce2.sql" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="reproduce2.sql" ZHJvcCB0YWJsZSBpZiBleGlzdHMgdDsKCnNldCBtYXhfcGFyYWxsZWxfd29ya2Vyc19wZXJf Z2F0aGVyID0gMDsKCmNyZWF0ZSB0YWJsZSB0KGEgYmlnaW50KSB3aXRoIChmaWxsZmFjdG9y ID0gMjUpOwoKaW5zZXJ0IGludG8gdCBzZWxlY3QgaSBmcm9tIGdlbmVyYXRlX3Nlcmllcygx LDEwMDAwMDAwKSBzKGkpOwoKY3JlYXRlIGluZGV4IGlkeF8xIG9uIHQgdXNpbmcgYnJpbiAo YSkgd2l0aCAocGFnZXNfcGVyX3JhbmdlID0gNCk7CgpjcmVhdGUgaW5kZXggaWR4XzIgb24g dCB1c2luZyBicmluIChhKTsKCmFuYWx5emUgdDsKClxkKwpcZGkrCgpleHBsYWluIGFuYWx5 emUgc2VsZWN0ICogZnJvbSB0IHdoZXJlIGEgPSAxMDAwOwoKZHJvcCBpbmRleCBpZHhfMjsK CmV4cGxhaW4gYW5hbHl6ZSBzZWxlY3QgKiBmcm9tIHQgd2hlcmUgYSA9IDEwMDA7Cg== --------------D524CD37E77B36A2883A7617 Content-Type: application/sql; name="reproducer.sql" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="reproducer.sql" ZHJvcCB0YWJsZSBpZiBleGlzdHMgdDsKCnNldCBtYXhfcGFyYWxsZWxfd29ya2Vyc19wZXJf Z2F0aGVyID0gMDsKCmNyZWF0ZSB0YWJsZSB0IChhIGludCwgYiBiaWdpbnQsIGMgdGV4dCkg d2l0aCAoZmlsbGZhY3RvciA9IDUwKTsKCmluc2VydCBpbnRvIHQgc2VsZWN0IDEwMDAwMDAg KiByYW5kb20oKSwgMTAwMDAwMCAqIHJhbmRvbSgpLCBtZDUoKDEwMDAwMDAgKiByYW5kb20o KSk6OmludDo6dGV4dCkgZnJvbSBnZW5lcmF0ZV9zZXJpZXMoMSwxMDAwMDAwKSBzKGkpOwoK c2VsZWN0IGNvdW50KCopIGZyb20gKHNlbGVjdCBkaXN0aW5jdCBiIGZyb20gdCB3aGVyZSBj dGlkIDwgJygxMjgsMCknOjp0aWQpIGZvbzsKCmNyZWF0ZSBpbmRleCBibG9vbV9hX2lkeCBv biB0IHVzaW5nIGJyaW4gKGEgaW50NF9ibG9vbV9vcHMobl9kaXN0aW5jdF9wZXJfcmFuZ2U9 NjAwMCkpOyAKCmFuYWx5emUgdDsKCmV4cGxhaW4gc2VsZWN0ICogZnJvbSB0IHdoZXJlIGEg PSAxMDAwOwoKY3JlYXRlIGluZGV4IGJyaW5faWR4IG9uIHQgdXNpbmcgYnJpbiAoYSk7CmFu YWx5emUgdDsKCmV4cGxhaW4gc2VsZWN0ICogZnJvbSB0IHdoZXJlIGEgPSAxMDAwOwoKZHJv cCBpbmRleCBicmluX2lkeDsKCmV4cGxhaW4gc2VsZWN0ICogZnJvbSB0IHdoZXJlIGEgPSAx MDAwOwo= --------------D524CD37E77B36A2883A7617-- ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Use of additional index columns in rows filtering @ 2023-02-15 15:18 Tom Lane <[email protected]> 0 siblings, 2 replies; 4+ messages in thread From: Tom Lane @ 2023-02-15 15:18 UTC (permalink / raw) To: Tomas Vondra <[email protected]>; +Cc: Maxim Ivanov <[email protected]>; pgsql-hackers Tomas Vondra <[email protected]> writes: > On 2/15/23 09:57, Maxim Ivanov wrote: >> TLDR; additional index column B specified in CREATE INDEX .. (A) >> INCLUDE(B) is not used to filter rows in queries like WHERE B = $1 >> ORDER BY A during IndexScan. https://dbfiddle.uk/iehtq44L > Seems doable, unless I'm missing some fatal issue. Partly this is lack of round tuits, but there's another significant issue: there very likely are index entries corresponding to dead heap rows. Applying random user-defined quals to values found in such rows could produce semantic anomalies; for example, divide-by-zero failures even though you deleted all the rows having a zero in that column. This isn't a problem for operators found in operator families, because we trust those to not have undesirable side effects like raising data-dependent errors. But it'd be an issue if we started to apply quals that aren't index quals directly to index rows before doing the heap liveness check. (And, of course, once you've fetched the heap row there's no point in having a special path for columns available from the index.) regards, tom lane ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Use of additional index columns in rows filtering @ 2023-02-15 17:01 Tomas Vondra <[email protected]> parent: Tom Lane <[email protected]> 1 sibling, 0 replies; 4+ messages in thread From: Tomas Vondra @ 2023-02-15 17:01 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Maxim Ivanov <[email protected]>; pgsql-hackers On 2/15/23 16:18, Tom Lane wrote: > Tomas Vondra <[email protected]> writes: >> On 2/15/23 09:57, Maxim Ivanov wrote: >>> TLDR; additional index column B specified in CREATE INDEX .. (A) >>> INCLUDE(B) is not used to filter rows in queries like WHERE B = $1 >>> ORDER BY A during IndexScan. https://dbfiddle.uk/iehtq44L > >> Seems doable, unless I'm missing some fatal issue. > > Partly this is lack of round tuits, but there's another significant > issue: there very likely are index entries corresponding to dead heap > rows. Applying random user-defined quals to values found in such rows > could produce semantic anomalies; for example, divide-by-zero failures > even though you deleted all the rows having a zero in that column. > > This isn't a problem for operators found in operator families, because > we trust those to not have undesirable side effects like raising > data-dependent errors. But it'd be an issue if we started to apply > quals that aren't index quals directly to index rows before doing > the heap liveness check. (And, of course, once you've fetched the > heap row there's no point in having a special path for columns > available from the index.) Sure, but we can do the same VM check as index-only scan, right? That would save some of the I/O to fetch the heap tuple, as long as the page is all-visible and the filter eliminates the tuples. It makes the costing a bit trickier, because it needs to consider both how many pages are all-visible and selectivity of the condition. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Use of additional index columns in rows filtering @ 2023-02-15 17:15 Maxim Ivanov <[email protected]> parent: Tom Lane <[email protected]> 1 sibling, 0 replies; 4+ messages in thread From: Maxim Ivanov @ 2023-02-15 17:15 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Tomas Vondra <[email protected]>; pgsql-hackers > This isn't a problem for operators found in operator families, because > we trust those to not have undesirable side effects like raising > data-dependent errors. But it'd be an issue if we started to apply > quals that aren't index quals directly to index rows before doing > the heap liveness check. (And, of course, once you've fetched the > heap row there's no point in having a special path for columns > available from the index.) Assuming operators are pure and don't have global side effects, is it possible to ignore any error during that check? If tuple is not visible it shouldn't matter, if it is visible then error will be reported by the same routine which does filtering now (ExecQual?). If not, then limiting this optimization to builtin ops is something I can live with :) ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2023-02-15 17:15 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2023-02-15 15:18 Re: Use of additional index columns in rows filtering Tom Lane <[email protected]> 2023-02-15 17:01 ` Re: Use of additional index columns in rows filtering Tomas Vondra <[email protected]> 2023-02-15 17:15 ` Re: Use of additional index columns in rows filtering Maxim Ivanov <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox