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

* walsender "wakeup storm" on PG16, likely because of bc971f4025c (Optimize walsender wake up logic using condition variables)
@ 2023-08-11 13:31  Tomas Vondra <[email protected]>
  0 siblings, 0 replies; 25+ messages in thread

From: Tomas Vondra @ 2023-08-11 13:31 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>; +Cc: Andres Freund <[email protected]>

Hi,

while working on some logical replication stuff, I noticed that on PG16
I often end up with a completely idle publisher (no user activity), that
however looks like this in top:

 ...  %CPU  COMMAND


 ...  17.9  postgres: walsender user test ::1(43064) START_REPLICATION


 ...  16.6  postgres: walsender user test ::1(43128) START_REPLICATION


 ...  15.0  postgres: walsender user test ::1(43202) START_REPLICATION


 ...   6.6  postgres: walsender user test ::1(43236) START_REPLICATION


 ...   5.3  postgres: walsender user test ::1(43086) START_REPLICATION


 ...   4.3  postgres: walsender user test ::1(43180) START_REPLICATION


 ...   3.7  postgres: walsender user test ::1(43052) START_REPLICATION


 ...   3.7  postgres: walsender user test ::1(43158) START_REPLICATION


 ...   3.0  postgres: walsender user test ::1(43108) START_REPLICATION


 ...   3.0  postgres: walsender user test ::1(43214) START_REPLICATION



That's an awful lot of CPU for a cluster doing essentially "nothing"
(there's no WAL to decode/replicate, etc.). It usually disappears after
a couple seconds, but sometimes it's a rather persistent state.

The profile from the walsender processes looks like this:

    --99.94%--XLogSendLogical
              |
              |--99.23%--XLogReadRecord
              |          XLogReadAhead
              |          XLogDecodeNextRecord
              |          ReadPageInternal
              |          logical_read_xlog_page
              |          |
              |          |--97.80%--WalSndWaitForWal
              |          |          |
              |          |          |--68.48%--WalSndWait

It seems to me the issue is in WalSndWait, which was reworked to use
ConditionVariableCancelSleep() in bc971f4025c. The walsenders end up
waking each other in a busy loop, until the timing changes just enough
to break the cycle.

I've been unable to reproduce this on PG15, and bc971f4025c seems like
the most significant change to WalSndWait, which is why I suspect it's
related to the issue.

Reproducing this is simple, create a publisher with multiple subscribers
(could even go to the same subscriber instance) and empty publications.
If you trigger a "noop" it's likely to cause this high memory usage:

---------------------------------------------------------------------
# init two clusters
pg_ctl -D data-publisher init
pg_ctl -D data-subscriber init

# change the parameters to allow 10 subscriptions
echo 'wal_level = logical' >> data-publisher/postgresql.conf
echo 'port = 5433' >> data-subscriber/postgresql.conf
echo 'max_worker_processes = 20' >> data-subscriber/postgresql.conf
echo 'max_logical_replication_workers = 20' >>
data-subscriber/postgresql.conf

# setup empty publication
createdb test
psql test -c "create publication p";

# setup 10 subscriptions
for i in `seq 1 10`; do
  createdb -p 5433 test$i
  psql -p 5433 test$i -c "create subscription s$i CONNECTION
'host=localhost port=5432 dbname=test' publication p";
done

# emit logical messages, which are almost noop, 5s apart
for i in `seq 1 10`; do
    psql test -c "select pg_logical_emit_message(false, 'x', 'x')";
    sleep 5;
done;
---------------------------------------------------------------------


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






^ permalink  raw  reply  [nested|flat] 25+ messages in thread


end of thread, other threads:[~2023-08-11 13:31 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]>
2023-08-11 13:31 walsender "wakeup storm" on PG16, likely because of bc971f4025c (Optimize walsender wake up logic using condition variables) Tomas Vondra <[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