public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 8/8] Consider selectivity in choose_bitmap_and 25+ messages / 2 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; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [PATCH 8/8] Consider selectivity in choose_bitmap_and @ 2021-03-11 11:57 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* Re: some validate_relation_kind() tidying @ 2026-02-19 08:48 Ashutosh Bapat <[email protected]> 0 siblings, 0 replies; 25+ messages in thread From: Ashutosh Bapat @ 2026-02-19 08:48 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers Hi Peter, On Wed, Feb 18, 2026 at 8:45 PM Peter Eisentraut <[email protected]> wrote: > > We have three different functions called validate_relation_kind(), namely in > > src/backend/access/index/indexam.c > src/backend/access/sequence/sequence.c > src/backend/access/table/table.c > > These all check which relkinds are permitted by index_open(), > sequence_open(), and table_open(), respectively, which are each wrappers > around relation_open() (which accepts any relkind). > It's better to use a different name for each of them as done in attached 0003. Same names can confuse code browsing tools like cscope and human reader alike. > I always found the one in table.c a little too mysterious, because it > just checks which relkinds it does *not* want, and so if you want to > know whether a particular relkind is suitable for table_open(), you need > to do additional research and check what all the possible relkinds are > and so on, and there is no real explanation why those choices were made. > I think it would be clearer and more robust and also more consistent > with the other ones if we flipped that around and listed the ones that > are acceptable and why. > The || should be &&. The bug shows up as an initdb failure running bootstrap script ... 2026-02-19 14:06:43.411 IST [197482] FATAL: cannot open relation "pg_type" 2026-02-19 14:06:43.411 IST [197482] DETAIL: This operation is not supported for tables. I think this is more future-proof. If a relkind gets added and needs to be in this list, we will notice it from the error. I think we should avoid mentioning specific relkinds in the comment as well since that list will need to be updated as the set of relkinds changes. Just mentioning the criteria should be enough. I have slightly improved the comment in the attached 0003. > Secondly, the sequence.c one was probably copied from the table.c one, > but I think we can make the error message a bit more direct by just > saying "... is not a sequence" instead of "cannot open relation". > +1. > These are the two attached patches. This is just something I found > while working on something else nearby. Attached are your two patches + bug fix in 0002 + my suggestions in 0003. -- Best Wishes, Ashutosh Bapat Attachments: [text/x-patch] v20260219-0001-Change-error-message-for-sequence-validate.patch (2.2K, ../../CAExHW5v9LTRuqqjS-N5g0n+cwGJLSf9+73ACgy4vFVzgzFb36A@mail.gmail.com/2-v20260219-0001-Change-error-message-for-sequence-validate.patch) download | inline diff: From e36f743ec00dd56d72db219c42743da01b8a75a7 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Wed, 18 Feb 2026 11:48:37 +0100 Subject: [PATCH v20260219 1/3] Change error message for sequence validate_relation_kind() We can just say "... is not a sequence" instead of the more complicated variant from before, which was probably copied from src/backend/access/table/table.c. Fix a typo in a comment in passing. --- src/backend/access/sequence/sequence.c | 7 +++---- src/test/regress/expected/sequence.out | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/backend/access/sequence/sequence.c b/src/backend/access/sequence/sequence.c index 106af1477e9..6897f8432d6 100644 --- a/src/backend/access/sequence/sequence.c +++ b/src/backend/access/sequence/sequence.c @@ -63,7 +63,7 @@ sequence_close(Relation relation, LOCKMODE lockmode) /* ---------------- * validate_relation_kind - check the relation's kind * - * Make sure relkind is from a sequence. + * Make sure relkind is a sequence. * ---------------- */ static inline void @@ -72,7 +72,6 @@ validate_relation_kind(Relation r) if (r->rd_rel->relkind != RELKIND_SEQUENCE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("cannot open relation \"%s\"", - RelationGetRelationName(r)), - errdetail_relkind_not_supported(r->rd_rel->relkind))); + errmsg("\"%s\" is not a sequence", + RelationGetRelationName(r)))); } diff --git a/src/test/regress/expected/sequence.out b/src/test/regress/expected/sequence.out index c4454e5b435..a0883b11007 100644 --- a/src/test/regress/expected/sequence.out +++ b/src/test/regress/expected/sequence.out @@ -313,8 +313,7 @@ ALTER SEQUENCE IF EXISTS sequence_test2 RESTART WITH 24 INCREMENT BY 4 MAXVALUE 36 MINVALUE 5 CYCLE; NOTICE: relation "sequence_test2" does not exist, skipping ALTER SEQUENCE serialTest1 CYCLE; -- error, not a sequence -ERROR: cannot open relation "serialtest1" -DETAIL: This operation is not supported for tables. +ERROR: "serialtest1" is not a sequence CREATE SEQUENCE sequence_test2 START WITH 32; CREATE SEQUENCE sequence_test4 INCREMENT BY -1; SELECT nextval('sequence_test2'); base-commit: 8354b9d6b602ea549bc8d85cb404771505662a7b -- 2.34.1 [text/x-patch] v20260219-0002-Flip-logic-in-table-validate_relation_kind.patch (2.0K, ../../CAExHW5v9LTRuqqjS-N5g0n+cwGJLSf9+73ACgy4vFVzgzFb36A@mail.gmail.com/3-v20260219-0002-Flip-logic-in-table-validate_relation_kind.patch) download | inline diff: From 51b3e27b35d21f6cd63be6bd34db61a32bd38154 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Wed, 18 Feb 2026 14:32:45 +0100 Subject: [PATCH v20260219 2/3] Flip logic in table validate_relation_kind It instead of checking which relkinds it shouldn't be, explicitly list the ones we accept. This is used to check which relkinds are accepted in table_open() and related functions. Before this change, figuring that out was always a few steps too complicated. This also makes changes for new relkinds more explicit instead of accidental. Finally, this makes this more aligned with the functions of the same name in src/backend/access/index/indexam.c and src/backend/access/sequence/sequence.c. --- src/backend/access/table/table.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/backend/access/table/table.c b/src/backend/access/table/table.c index fe330ae862a..0d8662a1452 100644 --- a/src/backend/access/table/table.c +++ b/src/backend/access/table/table.c @@ -131,15 +131,20 @@ table_close(Relation relation, LOCKMODE lockmode) /* ---------------- * validate_relation_kind - check the relation's kind * - * Make sure relkind is not index or composite type + * Make sure relkind is table-like. In particular, this excludes indexes + * and composite types, which cannot be read from in a query. * ---------------- */ static inline void validate_relation_kind(Relation r) { - if (r->rd_rel->relkind == RELKIND_INDEX || - r->rd_rel->relkind == RELKIND_PARTITIONED_INDEX || - r->rd_rel->relkind == RELKIND_COMPOSITE_TYPE) + if (r->rd_rel->relkind != RELKIND_RELATION && + r->rd_rel->relkind != RELKIND_SEQUENCE && + r->rd_rel->relkind != RELKIND_TOASTVALUE && + r->rd_rel->relkind != RELKIND_VIEW && + r->rd_rel->relkind != RELKIND_MATVIEW && + r->rd_rel->relkind != RELKIND_FOREIGN_TABLE && + r->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("cannot open relation \"%s\"", -- 2.34.1 [text/x-patch] v20260219-0003-Rename-validate_relation_kind.patch (5.5K, ../../CAExHW5v9LTRuqqjS-N5g0n+cwGJLSf9+73ACgy4vFVzgzFb36A@mail.gmail.com/4-v20260219-0003-Rename-validate_relation_kind.patch) download | inline diff: From 762e5708f9297b828b87c43909d52a778a46b0a0 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Thu, 19 Feb 2026 12:07:29 +0530 Subject: [PATCH v20260219 3/3] Rename validate_relation_kind() There are three static definitions of validate_relation_kind() in the codebase, one each in table.c, indexam.c and sequence.c, validating that the given relation is a table, an index or a sequence respectively. Compiler knows which definition to use where because they are static. But this could be confusing to a reader. Rename these functions so that their names reflect the kind of relation they are validating. While at it, also update the comments in table.c to clarify the definition of table-like relkinds so that we don't have to maintain the exclusion list as the set of relkinds undergoes changes. Author: Ashutosh Bapat <[email protected]> --- src/backend/access/index/indexam.c | 10 +++++----- src/backend/access/sequence/sequence.c | 8 ++++---- src/backend/access/table/table.c | 18 +++++++++--------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c index 4ed0508c605..43f64a0e721 100644 --- a/src/backend/access/index/indexam.c +++ b/src/backend/access/index/indexam.c @@ -107,7 +107,7 @@ do { \ static IndexScanDesc index_beginscan_internal(Relation indexRelation, int nkeys, int norderbys, Snapshot snapshot, ParallelIndexScanDesc pscan, bool temp_snap); -static inline void validate_relation_kind(Relation r); +static inline void validate_relation_as_index(Relation r); /* ---------------------------------------------------------------- @@ -136,7 +136,7 @@ index_open(Oid relationId, LOCKMODE lockmode) r = relation_open(relationId, lockmode); - validate_relation_kind(r); + validate_relation_as_index(r); return r; } @@ -159,7 +159,7 @@ try_index_open(Oid relationId, LOCKMODE lockmode) if (!r) return NULL; - validate_relation_kind(r); + validate_relation_as_index(r); return r; } @@ -188,13 +188,13 @@ index_close(Relation relation, LOCKMODE lockmode) } /* ---------------- - * validate_relation_kind - check the relation's kind + * validate_relation_as_index * * Make sure relkind is an index or a partitioned index. * ---------------- */ static inline void -validate_relation_kind(Relation r) +validate_relation_as_index(Relation r) { if (r->rd_rel->relkind != RELKIND_INDEX && r->rd_rel->relkind != RELKIND_PARTITIONED_INDEX) diff --git a/src/backend/access/sequence/sequence.c b/src/backend/access/sequence/sequence.c index 6897f8432d6..81f46163749 100644 --- a/src/backend/access/sequence/sequence.c +++ b/src/backend/access/sequence/sequence.c @@ -24,7 +24,7 @@ #include "access/sequence.h" #include "utils/rel.h" -static inline void validate_relation_kind(Relation r); +static inline void validate_relation_as_sequence(Relation r); /* ---------------- * sequence_open - open a sequence relation by relation OID @@ -40,7 +40,7 @@ sequence_open(Oid relationId, LOCKMODE lockmode) r = relation_open(relationId, lockmode); - validate_relation_kind(r); + validate_relation_as_sequence(r); return r; } @@ -61,13 +61,13 @@ sequence_close(Relation relation, LOCKMODE lockmode) } /* ---------------- - * validate_relation_kind - check the relation's kind + * validate_relation_as_sequence * * Make sure relkind is a sequence. * ---------------- */ static inline void -validate_relation_kind(Relation r) +validate_relation_as_sequence(Relation r) { if (r->rd_rel->relkind != RELKIND_SEQUENCE) ereport(ERROR, diff --git a/src/backend/access/table/table.c b/src/backend/access/table/table.c index 0d8662a1452..00288ac9c70 100644 --- a/src/backend/access/table/table.c +++ b/src/backend/access/table/table.c @@ -25,7 +25,7 @@ #include "access/table.h" #include "utils/rel.h" -static inline void validate_relation_kind(Relation r); +static inline void validate_relation_as_table(Relation r); /* ---------------- * table_open - open a table relation by relation OID @@ -43,7 +43,7 @@ table_open(Oid relationId, LOCKMODE lockmode) r = relation_open(relationId, lockmode); - validate_relation_kind(r); + validate_relation_as_table(r); return r; } @@ -67,7 +67,7 @@ try_table_open(Oid relationId, LOCKMODE lockmode) if (!r) return NULL; - validate_relation_kind(r); + validate_relation_as_table(r); return r; } @@ -86,7 +86,7 @@ table_openrv(const RangeVar *relation, LOCKMODE lockmode) r = relation_openrv(relation, lockmode); - validate_relation_kind(r); + validate_relation_as_table(r); return r; } @@ -108,7 +108,7 @@ table_openrv_extended(const RangeVar *relation, LOCKMODE lockmode, r = relation_openrv_extended(relation, lockmode, missing_ok); if (r) - validate_relation_kind(r); + validate_relation_as_table(r); return r; } @@ -129,14 +129,14 @@ table_close(Relation relation, LOCKMODE lockmode) } /* ---------------- - * validate_relation_kind - check the relation's kind + * validate_relation_as_table * - * Make sure relkind is table-like. In particular, this excludes indexes - * and composite types, which cannot be read from in a query. + * Make sure relkind is table-like i.e. something which can be referenced + * directly in the query. * ---------------- */ static inline void -validate_relation_kind(Relation r) +validate_relation_as_table(Relation r) { if (r->rd_rel->relkind != RELKIND_RELATION && r->rd_rel->relkind != RELKIND_SEQUENCE && -- 2.34.1 ^ permalink raw reply [nested|flat] 25+ messages in thread
end of thread, other threads:[~2026-02-19 08:48 UTC | newest] Thread overview: 25+ 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]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2021-03-11 11:57 [PATCH 8/8] Consider selectivity in choose_bitmap_and Tomas Vondra <[email protected]> 2026-02-19 08:48 Re: some validate_relation_kind() tidying Ashutosh Bapat <[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