public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 8/8] Consider selectivity in choose_bitmap_and 3+ messages / 3 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; 3+ 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] 3+ messages in thread
* Re: proposal - get_extension_version function @ 2023-03-08 19:17 Tom Lane <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Tom Lane @ 2023-03-08 19:17 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: Pavel Stehule <[email protected]>; PostgreSQL Hackers <[email protected]> Jacob Champion <[email protected]> writes: > On Wed, Mar 8, 2023 at 10:49 AM Tom Lane <[email protected]> wrote: >> This is a bad idea. How will you do extension upgrades, if the new .so >> won't run till you apply the extension upgrade script but the old .so >> malfunctions as soon as you do? > Which upgrade paths allow you to have an old .so with a new version > number? I didn't realize that was an issue. More usually, it's the other way around: new .so but SQL objects not upgraded yet. That's typical in a pg_upgrade to a new major version, where the new installation may have a newer extension .so than the old one did. You can't run ALTER EXTENSION UPGRADE if the new .so refuses to load with the old SQL objects ... which AFAICS is exactly what Pavel's sketch would do. If you have old .so and new SQL objects, it's likely that at least some of those new objects won't work --- but it's good to not break any more functionality than you have to. That's why I suggest managing the compatibility checks on a per-function level rather than trying to have an overall version check. regards, tom lane ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: proposal - get_extension_version function @ 2023-03-08 21:10 Jacob Champion <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Jacob Champion @ 2023-03-08 21:10 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Pavel Stehule <[email protected]>; PostgreSQL Hackers <[email protected]> On Wed, Mar 8, 2023 at 11:18 AM Tom Lane <[email protected]> wrote: > > Jacob Champion <[email protected]> writes: > > On Wed, Mar 8, 2023 at 10:49 AM Tom Lane <[email protected]> wrote: > >> This is a bad idea. How will you do extension upgrades, if the new .so > >> won't run till you apply the extension upgrade script but the old .so > >> malfunctions as soon as you do? > > > Which upgrade paths allow you to have an old .so with a new version > > number? I didn't realize that was an issue. > > More usually, it's the other way around: new .so but SQL objects not > upgraded yet. That's typical in a pg_upgrade to a new major version, > where the new installation may have a newer extension .so than the > old one did. That's the opposite case though; I think the expectation of backwards compatibility from C to SQL is very different from (infinite?) forwards compatibility from C to SQL. > If you have old .so and new SQL objects, it's likely that at least > some of those new objects won't work --- but it's good to not break > any more functionality than you have to. To me it doesn't seem like a partial break is safer than refusing to execute in the face of old-C-and-new-SQL -- assuming it's safe at all? A bailout seems pretty reasonable in that case. Thanks, --Jacob ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-03-08 21:10 UTC | newest] Thread overview: 3+ 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-03-08 19:17 Re: proposal - get_extension_version function Tom Lane <[email protected]> 2023-03-08 21:10 ` Re: proposal - get_extension_version function Jacob Champion <[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