public inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH 8/8] Consider selectivity in choose_bitmap_and
26+ messages / 3 participants
[nested] [flat]

* [PATCH 8/8] Consider selectivity in choose_bitmap_and
@ 2021-03-11 11:57 Tomas Vondra <[email protected]>
  0 siblings, 0 replies; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ messages in thread

* Lowering the default wal_blocksize to 4K
@ 2023-10-09 23:08 Andres Freund <[email protected]>
  2023-10-10 10:57 ` Re: Lowering the default wal_blocksize to 4K Matthias van de Meent <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Andres Freund @ 2023-10-09 23:08 UTC (permalink / raw)
  To: pgsql-hackers; Heikki Linnakangas <[email protected]>; Robert Haas <[email protected]>; Thomas Munro <[email protected]>; Matthias van de Meent <[email protected]>

Hi,

I've mentioned this to a few people before, but forgot to start an actual
thread. So here we go:

I think we should lower the default wal_blocksize / XLOG_BLCKSZ to 4096, from
the current 8192.  The reason is that

a) We don't gain much from a blocksize above 4096, as we already do one write
   all the pending WAL data in one go (except when at the tail of
   wal_buffers). We *do* incur more overhead for page headers, but compared to
   the actual WAL data it is not a lot (~0.29% of space is page headers 8192
   vs 0.59% with 4096).

b) Writing 8KB when we we have to flush a partially filled buffer can
   substantially increase write amplification. In a transactional workload,
   this will often double the write volume.

Currently disks mostly have 4096 bytes as their "sector size". Sometimes
that's exposed directly, sometimes they can also write in 512 bytes, but that
internally requires a read-modify-write operation.


For some example numbers, I ran a very simple insert workload with a varying
number of clients with both a wal_blocksize=4096 and wal_blocksize=8192
cluster, and measured the amount of bytes written before/after.  The table was
recreated before each run, followed by a checkpoint and the benchmark. Here I
ran the inserts only for 15s each, because the results don't change
meaningfully with longer runs.


With XLOG_BLCKSZ=8192

clients	     tps    disk bytes written
1	     667		 81296
2	     739		 89796
4	    1446		 89208
8	    2858		 90858
16	    5775		 96928
32	   11920		115351
64	   23686		135244
128	   46001		173390
256	   88833		239720
512	  146208		335669


With XLOG_BLCKSZ=4096

clients	     tps    disk bytes written
1	     751		 46838
2	     773		 47936
4	    1512		 48317
8	    3143		 52584
16	    6221		 59097
32	   12863		 73776
64	   25652		 98792
128	   48274		133330
256	   88969		200720
512	  146298		298523


This is on a not-that-fast NVMe SSD (Samsung SSD 970 PRO 1TB).


It's IMO quite interesting that even at the higher client counts, the number
of bytes written don't reach parity.


On a stripe of two very fast SSDs:

With XLOG_BLCKSZ=8192

clients	     tps    disk bytes written
1	   23786		2893392
2	   38515		4683336
4	   63436		4688052
8	  106618		4618760
16	  177905		4384360
32	  254890		3890664
64	  297113		3031568
128	  299878		2297808
256	  308774		1935064
512	  292515		1630408


With XLOG_BLCKSZ=4096

clients	     tps    disk bytes written
1	   25742		1586748
2	   43578		2686708
4	   62734		2613856
8	  116217		2809560
16	  200802		2947580
32	  269268		2461364
64	  323195		2042196
128	  317160		1550364
256	  309601		1285744
512	  292063		1103816

It's fun to see how the total number of writes *decreases* at higher
concurrency, because it becomes more likely that pages are filled completely.


One thing I noticed is that our auto-configuration of wal_buffers leads to
different wal_buffers settings for different XLOG_BLCKSZ, which doesn't seem
great.


Performing the same COPY workload (1024 files, split across N clients) for
both settings shows no performance difference, but a very slight increase in
total bytes written (about 0.25%, which is roughly what I'd expect).


Personally I'd say the slight increase in WAL volume is more than outweighed
by the increase in throughput and decrease in bytes written.


There's an alternative approach we could take, which is to write in 4KB
increments, while keeping 8KB pages. With the current format that's not
obviously a bad idea. But given there aren't really advantages in 8KB WAL
pages, it seems we should just go for 4KB?

Greetings,

Andres Freund






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

* Re: Lowering the default wal_blocksize to 4K
  2023-10-09 23:08 Lowering the default wal_blocksize to 4K Andres Freund <[email protected]>
@ 2023-10-10 10:57 ` Matthias van de Meent <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: Matthias van de Meent @ 2023-10-10 10:57 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: pgsql-hackers; Heikki Linnakangas <[email protected]>; Robert Haas <[email protected]>; Thomas Munro <[email protected]>

On Tue, 10 Oct 2023 at 01:08, Andres Freund <[email protected]> wrote:
>
> Hi,
>
> I've mentioned this to a few people before, but forgot to start an actual
> thread. So here we go:
>
> I think we should lower the default wal_blocksize / XLOG_BLCKSZ to 4096, from
> the current 8192.

Seems like a good idea.

> It's IMO quite interesting that even at the higher client counts, the number
> of bytes written don't reach parity.
>
> It's fun to see how the total number of writes *decreases* at higher
> concurrency, because it becomes more likely that pages are filled completely.

With higher client counts and short transactions I think it is not too
unexpected to see commit_delay+commit_siblings configured. Did you
measure the impact of this change on such configurations?

> One thing I noticed is that our auto-configuration of wal_buffers leads to
> different wal_buffers settings for different XLOG_BLCKSZ, which doesn't seem
> great.

Hmm.

> Performing the same COPY workload (1024 files, split across N clients) for
> both settings shows no performance difference, but a very slight increase in
> total bytes written (about 0.25%, which is roughly what I'd expect).
>
> Personally I'd say the slight increase in WAL volume is more than outweighed
> by the increase in throughput and decrease in bytes written.

Agreed.

> There's an alternative approach we could take, which is to write in 4KB
> increments, while keeping 8KB pages. With the current format that's not
> obviously a bad idea. But given there aren't really advantages in 8KB WAL
> pages, it seems we should just go for 4KB?

It is not just the disk overhead of blocks, but we also maintain some
other data (currently in the form of XLogRecPtrs) in memory for each
WAL buffer, the overhead of which will also increase when we increase
the number of XLog pages per MB of WAL that we cache.
Additionally, highly concurrent workloads with transactions that write
a high multiple of XLOG_BLCKSZ bytes to WAL may start to see increased
overhead due to the .25% additional WAL getting written and a doubling
of the number of XLog pages being touched (both initialization and the
smaller memcpy for records that would now cross an extra page
boundary).

However, for all of these issues I doubt that they actually matter
much in the grand scheme of things, so I definitely wouldn't mind
moving to 4KiB XLog pages.

Kind regards,

Matthias van de Meent
Neon (https://neon.tech)






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

end of thread, other threads:[~2023-10-10 10:57 UTC | newest]

Thread overview: 26+ 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-10-09 23:08 Lowering the default wal_blocksize to 4K Andres Freund <[email protected]>
2023-10-10 10:57 ` Re: Lowering the default wal_blocksize to 4K Matthias van de Meent <[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