public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 8/8] Consider selectivity in choose_bitmap_and
28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ 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; 28+ 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] 28+ messages in thread
* Re: Trigger more frequent autovacuums of heavy insert tables
@ 2024-10-24 19:51 Greg Sabino Mullane <[email protected]>
2024-10-25 15:14 ` Re: Trigger more frequent autovacuums of heavy insert tables Melanie Plageman <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Greg Sabino Mullane @ 2024-10-24 19:51 UTC (permalink / raw)
To: Melanie Plageman <[email protected]>; +Cc: pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Peter Geoghegan <[email protected]>; David Rowley <[email protected]>
I really appreciate all the work to make vacuum better. Anything that helps
our problem of autovacuum not scaling well for large tables is a win.
I'm not overly familiar with this part of the code base, but here are some
questions/ideas:
+ /*
+ * Every block marked all-frozen in the VM must also be marked
+ * all-visible.
+ */
+ if (new_rel_allfrozen > new_rel_allvisible)
+ new_rel_allfrozen = new_rel_allvisible;
+
Maybe tweak either the comment, or the code, as I read that comment as
meaning:
if (new_rel_allfrozen > new_rel_allvisible)
new_ral_allvisible = new_rel_allfrozen;
+ /*
+ * If we are modifying relallvisible manually, it
is not clear
+ * what relallfrozen value would make sense.
Therefore, set it to
+ * -1, or unknown. It will be updated the next time
these fields
+ * are updated.
+ */
+ replaces[ncols] = Anum_pg_class_relallfrozen;
+ values[ncols] = Int32GetDatum(-1);
Do we need some extra checks later on when we are actually using this to
prevent negative numbers in the calculations? It's only going to make
pcnt_unfrozen something like 1.0001 but still might want to skip that.
In autovacuum.c, seems we could simplify some of the logic there to this?:
if (relpages > 0 && reltuples > 0) {
relallfrozen = classForm->relallfrozen;
relallvisible = classForm->relallvisible;
if (relallvisible > relpages)
relallvisible = relpages;
if (relallfrozen > relallvisible)
relallfrozen = relallvisible;
pcnt_unfrozen = 1 - ((float4) relallfrozen / relpages);
}
vacinsthresh = (float4) vac_ins_base_thresh + vac_ins_scale_factor *
reltuples * pcnt_unfrozen;
Again, I'm not clear under what circumstances will relallvisible > relpages?
Cheers,
Greg
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Trigger more frequent autovacuums of heavy insert tables
2024-10-24 19:51 Re: Trigger more frequent autovacuums of heavy insert tables Greg Sabino Mullane <[email protected]>
@ 2024-10-25 15:14 ` Melanie Plageman <[email protected]>
2025-01-16 21:43 ` Re: Trigger more frequent autovacuums of heavy insert tables Melanie Plageman <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Melanie Plageman @ 2024-10-25 15:14 UTC (permalink / raw)
To: Greg Sabino Mullane <[email protected]>; +Cc: pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Peter Geoghegan <[email protected]>; David Rowley <[email protected]>
Thanks for the review!
On Thu, Oct 24, 2024 at 3:51 PM Greg Sabino Mullane <[email protected]> wrote:
>
> I really appreciate all the work to make vacuum better. Anything that helps our problem of autovacuum not scaling well for large tables is a win.
>
> I'm not overly familiar with this part of the code base, but here are some questions/ideas:
>
> + /*
> + * Every block marked all-frozen in the VM must also be marked
> + * all-visible.
> + */
> + if (new_rel_allfrozen > new_rel_allvisible)
> + new_rel_allfrozen = new_rel_allvisible;
> +
>
> Maybe tweak either the comment, or the code, as I read that comment as meaning:
>
> if (new_rel_allfrozen > new_rel_allvisible)
> new_ral_allvisible = new_rel_allfrozen;
I've updated it. An all-frozen block must also be all-visible. But not
all-visible blocks are all-frozen
> + /*
> + * If we are modifying relallvisible manually, it is not clear
> + * what relallfrozen value would make sense. Therefore, set it to
> + * -1, or unknown. It will be updated the next time these fields
> + * are updated.
> + */
> + replaces[ncols] = Anum_pg_class_relallfrozen;
> + values[ncols] = Int32GetDatum(-1);
>
> Do we need some extra checks later on when we are actually using this to prevent negative numbers in the calculations? It's only going to make pcnt_unfrozen something like 1.0001 but still might want to skip that.
Great point! I've added this
> In autovacuum.c, seems we could simplify some of the logic there to this?:
>
> if (relpages > 0 && reltuples > 0) {
>
> relallfrozen = classForm->relallfrozen;
> relallvisible = classForm->relallvisible;
>
> if (relallvisible > relpages)
> relallvisible = relpages;
>
> if (relallfrozen > relallvisible)
> relallfrozen = relallvisible;
>
> pcnt_unfrozen = 1 - ((float4) relallfrozen / relpages);
>
> }
> vacinsthresh = (float4) vac_ins_base_thresh + vac_ins_scale_factor * reltuples * pcnt_unfrozen;
I've done something similar to this in attached v2.
> Again, I'm not clear under what circumstances will relallvisible > relpages?
I think this is mostly if someone manually updated the relation stats,
so we clamp it for safety.
- Melanie
Attachments:
[text/x-patch] v3-0002-Trigger-more-frequent-autovacuums-with-relallfroz.patch (4.7K, ../../CAAKRu_YmQtqnhBxQTDietJCSG0NHuGxmA33ty7KbP6zUA+=pVw@mail.gmail.com/2-v3-0002-Trigger-more-frequent-autovacuums-with-relallfroz.patch)
download | inline diff:
From c2e29150e923f9782ce24a7a4e7d6f2d7445b543 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Fri, 4 Oct 2024 17:06:04 -0400
Subject: [PATCH v3 2/2] Trigger more frequent autovacuums with relallfrozen
Calculate the insert threshold for triggering an autovacuum of a
relation based on the number of unfrozen pages. By only considering the
"active" (unfrozen) portion of the table when calculating how many
tuples to add to the insert threshold, we can trigger more frequent
vacuums of insert-heavy tables and increase the chances of vacuuming
those pages when they still reside in shared buffers.
Author: Melanie Plageman
Reviewed-by: Greg Sabino Mullane
---
doc/src/sgml/config.sgml | 4 +-
src/backend/postmaster/autovacuum.c | 37 ++++++++++++++++++-
src/backend/utils/misc/postgresql.conf.sample | 4 +-
3 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index dc401087dc6..3bd22f7f1e7 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8710,10 +8710,10 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
</term>
<listitem>
<para>
- Specifies a fraction of the table size to add to
+ Specifies a fraction of the active (unfrozen) table size to add to
<varname>autovacuum_vacuum_insert_threshold</varname>
when deciding whether to trigger a <command>VACUUM</command>.
- The default is 0.2 (20% of table size).
+ The default is 0.2 (20% of active table size).
This parameter can only be set in the <filename>postgresql.conf</filename>
file or on the server command line;
but the setting can be overridden for individual tables by
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index dc3cf87abab..364b46f672d 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2922,7 +2922,12 @@ relation_needs_vacanalyze(Oid relid,
{
bool force_vacuum;
bool av_enabled;
- float4 reltuples; /* pg_class.reltuples */
+
+ /* From pg_class */
+ float4 reltuples;
+ int32 relpages;
+ int32 relallfrozen;
+ int32 relallvisible;
/* constants from reloptions or GUC variables */
int vac_base_thresh,
@@ -3030,6 +3035,10 @@ relation_needs_vacanalyze(Oid relid,
*/
if (PointerIsValid(tabentry) && AutoVacuumingActive())
{
+ float4 pcnt_unfrozen = 1;
+
+ relpages = classForm->relpages;
+ relallfrozen = classForm->relallfrozen;
reltuples = classForm->reltuples;
vactuples = tabentry->dead_tuples;
instuples = tabentry->ins_since_vacuum;
@@ -3039,8 +3048,32 @@ relation_needs_vacanalyze(Oid relid,
if (reltuples < 0)
reltuples = 0;
+ /*
+ * If the table has been vacuumed and we have reliable data for
+ * relallfrozen and relallvisible, calculate the unfrozen percentage
+ * of the table to modify insert scale factor. This helps us decide
+ * whether or not to vacuum an insert-heavy table based on the number
+ * of inserts to the "active" part of the table.
+ *
+ * If relallfrozen is -1, that means relallvisible was updated
+ * manually and we can't rely on relallfrozen.
+ */
+ if (relpages > 0 && reltuples > 0 && relallfrozen > -1)
+ {
+ relallvisible = classForm->relallvisible;
+
+ if (relallvisible > relpages)
+ relallvisible = relpages;
+
+ if (relallfrozen > relallvisible)
+ relallfrozen = relallvisible;
+
+ pcnt_unfrozen = 1 - ((float4) relallfrozen / relpages);
+ }
+
vacthresh = (float4) vac_base_thresh + vac_scale_factor * reltuples;
- vacinsthresh = (float4) vac_ins_base_thresh + vac_ins_scale_factor * reltuples;
+ vacinsthresh = (float4) vac_ins_base_thresh +
+ vac_ins_scale_factor * reltuples * pcnt_unfrozen;
anlthresh = (float4) anl_base_thresh + anl_scale_factor * reltuples;
/*
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 39a3ac23127..a66f16e838a 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -670,8 +670,8 @@
#autovacuum_analyze_threshold = 50 # min number of row updates before
# analyze
#autovacuum_vacuum_scale_factor = 0.2 # fraction of table size before vacuum
-#autovacuum_vacuum_insert_scale_factor = 0.2 # fraction of inserts over table
- # size before insert vacuum
+#autovacuum_vacuum_insert_scale_factor = 0.2 # fraction of inserts over active
+ # table size before insert vacuum
#autovacuum_analyze_scale_factor = 0.1 # fraction of table size before analyze
#autovacuum_freeze_max_age = 200000000 # maximum XID age before forced vacuum
# (change requires restart)
--
2.34.1
[text/x-patch] v3-0001-Add-relallfrozen-to-pg_class.patch (15.3K, ../../CAAKRu_YmQtqnhBxQTDietJCSG0NHuGxmA33ty7KbP6zUA+=pVw@mail.gmail.com/3-v3-0001-Add-relallfrozen-to-pg_class.patch)
download | inline diff:
From caab36b29b9a9e2582283279d2e9b0fa404307ae Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Fri, 25 Oct 2024 10:43:39 -0400
Subject: [PATCH v3 1/2] Add relallfrozen to pg_class
Add relallfrozen, an estimate of the number of pages marked all-frozen
in the visibility map.
pg_class already has relallvisible, an estimate of the number of pages
in the relation marked all-visible in the visibility map. This is used
primarily for planning.
relallfrozen, however, is useful for estimating the outstanding number
of all-visible but not all-frozen pages in the relation for the purposes
of scheduling manual VACUUMs and tuning vacuum freeze parameters.
In the future, this field could be used to trigger more frequent vacuums
on insert-focused workloads with significant volume of frozen data.
Author: Melanie Plageman
Reviewed-by: Greg Sabino Mullane
---
doc/src/sgml/catalogs.sgml | 20 ++++++++++++++++++
src/backend/access/heap/vacuumlazy.c | 17 ++++++++++++----
src/backend/catalog/heap.c | 2 ++
src/backend/catalog/index.c | 27 +++++++++++++++++--------
src/backend/commands/analyze.c | 12 +++++------
src/backend/commands/cluster.c | 5 +++++
src/backend/commands/vacuum.c | 6 ++++++
src/backend/statistics/relation_stats.c | 20 +++++++++++++++---
src/backend/utils/cache/relcache.c | 2 ++
src/include/catalog/catversion.h | 2 +-
src/include/catalog/pg_class.h | 3 +++
src/include/commands/vacuum.h | 1 +
12 files changed, 95 insertions(+), 22 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 964c819a02d..392a1f71eb0 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -2066,6 +2066,26 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
</para></entry>
</row>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>relallfrozen</structfield> <type>int4</type>
+ </para>
+ <para>
+ Number of pages that are marked all-frozen in the table's visibility
+ map. This is only an estimate used for triggering autovacuums. It is
+ updated by <link linkend="sql-vacuum"><command>VACUUM</command></link>,
+ <link linkend="sql-analyze"><command>ANALYZE</command></link>,
+ and a few DDL commands such as
+ <link linkend="sql-createindex"><command>CREATE INDEX</command></link>.
+ <structfield>relallfrozen</structfield> must be less than or equal to
+ <strutfield>relallvisible</structfield> as an all-frozen page must be
+ all-visible. If <structfield>relallvisible</structfield> was updated
+ manually, <structfield>relallfrozen</structfield> will be -1 until the
+ next time they are updated.
+ </para></entry>
+ </row>
+
+
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>reltoastrelid</structfield> <type>oid</type>
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index d82aa3d4896..776d1673bbb 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -303,7 +303,8 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
minmulti_updated;
BlockNumber orig_rel_pages,
new_rel_pages,
- new_rel_allvisible;
+ new_rel_allvisible,
+ new_rel_allfrozen;
PGRUsage ru0;
TimestampTz starttime = 0;
PgStat_Counter startreadtime = 0,
@@ -558,10 +559,17 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
* pg_class.relpages to
*/
new_rel_pages = vacrel->rel_pages; /* After possible rel truncation */
- visibilitymap_count(rel, &new_rel_allvisible, NULL);
+ visibilitymap_count(rel, &new_rel_allvisible, &new_rel_allfrozen);
if (new_rel_allvisible > new_rel_pages)
new_rel_allvisible = new_rel_pages;
+ /*
+ * An all-frozen block _must_ be all-visible. As such, clamp the count of
+ * all-frozen blocks to the count of all-visible blocks.
+ */
+ if (new_rel_allfrozen > new_rel_allvisible)
+ new_rel_allfrozen = new_rel_allvisible;
+
/*
* Now actually update rel's pg_class entry.
*
@@ -570,7 +578,8 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
* scan every page that isn't skipped using the visibility map.
*/
vac_update_relstats(rel, new_rel_pages, vacrel->new_live_tuples,
- new_rel_allvisible, vacrel->nindexes > 0,
+ new_rel_allvisible, new_rel_allfrozen,
+ vacrel->nindexes > 0,
vacrel->NewRelfrozenXid, vacrel->NewRelminMxid,
&frozenxid_updated, &minmulti_updated, false);
@@ -3100,7 +3109,7 @@ update_relstats_all_indexes(LVRelState *vacrel)
vac_update_relstats(indrel,
istat->num_pages,
istat->num_index_tuples,
- 0,
+ 0, 0,
false,
InvalidTransactionId,
InvalidMultiXactId,
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 0078a12f26e..d150d2b9a72 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -920,6 +920,7 @@ InsertPgClassTuple(Relation pg_class_desc,
values[Anum_pg_class_relpages - 1] = Int32GetDatum(rd_rel->relpages);
values[Anum_pg_class_reltuples - 1] = Float4GetDatum(rd_rel->reltuples);
values[Anum_pg_class_relallvisible - 1] = Int32GetDatum(rd_rel->relallvisible);
+ values[Anum_pg_class_relallfrozen - 1] = Int32GetDatum(rd_rel->relallfrozen);
values[Anum_pg_class_reltoastrelid - 1] = ObjectIdGetDatum(rd_rel->reltoastrelid);
values[Anum_pg_class_relhasindex - 1] = BoolGetDatum(rd_rel->relhasindex);
values[Anum_pg_class_relisshared - 1] = BoolGetDatum(rd_rel->relisshared);
@@ -990,6 +991,7 @@ AddNewRelationTuple(Relation pg_class_desc,
new_rel_reltup->relpages = 0;
new_rel_reltup->reltuples = -1;
new_rel_reltup->relallvisible = 0;
+ new_rel_reltup->relallfrozen = 0;
/* Sequences always have a known size */
if (relkind == RELKIND_SEQUENCE)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 9162b9f81a2..6f1bc159ecd 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2795,8 +2795,8 @@ FormIndexDatum(IndexInfo *indexInfo,
* hasindex: set relhasindex to this value
* reltuples: if >= 0, set reltuples to this value; else no change
*
- * If reltuples >= 0, relpages and relallvisible are also updated (using
- * RelationGetNumberOfBlocks() and visibilitymap_count()).
+ * If reltuples >= 0, relpages, relallvisible, and relallfrozen are also
+ * updated (using RelationGetNumberOfBlocks() and visibilitymap_count()).
*
* NOTE: an important side-effect of this operation is that an SI invalidation
* message is sent out to all backends --- including me --- causing relcache
@@ -2841,8 +2841,8 @@ index_update_stats(Relation rel,
* transaction could still fail before committing. Setting relhasindex
* true is safe even if there are no indexes (VACUUM will eventually fix
* it). And of course the new relpages and reltuples counts are correct
- * regardless. However, we don't want to change relpages (or
- * relallvisible) if the caller isn't providing an updated reltuples
+ * regardless. However, we don't want to change relpages (or relallvisible
+ * and relallfrozen) if the caller isn't providing an updated reltuples
* count, because that would bollix the reltuples/relpages ratio which is
* what's really important.
*/
@@ -2888,12 +2888,18 @@ index_update_stats(Relation rel,
if (reltuples >= 0 && !IsBinaryUpgrade)
{
BlockNumber relpages = RelationGetNumberOfBlocks(rel);
- BlockNumber relallvisible;
+ BlockNumber relallvisible = 0;
+ BlockNumber relallfrozen = 0;
+ /* don't bother for indexes */
if (rd_rel->relkind != RELKIND_INDEX)
- visibilitymap_count(rel, &relallvisible, NULL);
- else /* don't bother for indexes */
- relallvisible = 0;
+ {
+ visibilitymap_count(rel, &relallvisible, &relallfrozen);
+
+ /* Every all-frozen page must also be set all-visible in the VM */
+ if (relallfrozen > relallvisible)
+ relallfrozen = relallvisible;
+ }
if (rd_rel->relpages != (int32) relpages)
{
@@ -2910,6 +2916,11 @@ index_update_stats(Relation rel,
rd_rel->relallvisible = (int32) relallvisible;
dirty = true;
}
+ if (rd_rel->relallfrozen != (int32) relallfrozen)
+ {
+ rd_rel->relallfrozen = (int32) relallfrozen;
+ dirty = true;
+ }
}
/*
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 38fb4c3ef23..0928592272e 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -630,12 +630,11 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
*/
if (!inh)
{
- BlockNumber relallvisible;
+ BlockNumber relallvisible = 0;
+ BlockNumber relallfrozen = 0;
if (RELKIND_HAS_STORAGE(onerel->rd_rel->relkind))
- visibilitymap_count(onerel, &relallvisible, NULL);
- else
- relallvisible = 0;
+ visibilitymap_count(onerel, &relallvisible, &relallfrozen);
/*
* Update pg_class for table relation. CCI first, in case acquirefunc
@@ -646,6 +645,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
relpages,
totalrows,
relallvisible,
+ relallfrozen,
hasindex,
InvalidTransactionId,
InvalidMultiXactId,
@@ -662,7 +662,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
vac_update_relstats(Irel[ind],
RelationGetNumberOfBlocks(Irel[ind]),
totalindexrows,
- 0,
+ 0, 0,
false,
InvalidTransactionId,
InvalidMultiXactId,
@@ -678,7 +678,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
*/
CommandCounterIncrement();
vac_update_relstats(onerel, -1, totalrows,
- 0, hasindex, InvalidTransactionId,
+ 0, 0, hasindex, InvalidTransactionId,
InvalidMultiXactId,
NULL, NULL,
in_outer_xact);
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 78f96789b0e..43763550668 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1224,6 +1224,7 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
int32 swap_pages;
float4 swap_tuples;
int32 swap_allvisible;
+ int32 swap_allfrozen;
swap_pages = relform1->relpages;
relform1->relpages = relform2->relpages;
@@ -1236,6 +1237,10 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
swap_allvisible = relform1->relallvisible;
relform1->relallvisible = relform2->relallvisible;
relform2->relallvisible = swap_allvisible;
+
+ swap_allfrozen = relform1->relallfrozen;
+ relform1->relallfrozen = relform2->relallfrozen;
+ relform2->relallfrozen = swap_allfrozen;
}
/*
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index ac8f5d9c259..7ed4df509c5 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1410,6 +1410,7 @@ void
vac_update_relstats(Relation relation,
BlockNumber num_pages, double num_tuples,
BlockNumber num_all_visible_pages,
+ BlockNumber num_all_frozen_pages,
bool hasindex, TransactionId frozenxid,
MultiXactId minmulti,
bool *frozenxid_updated, bool *minmulti_updated,
@@ -1459,6 +1460,11 @@ vac_update_relstats(Relation relation,
pgcform->relallvisible = (int32) num_all_visible_pages;
dirty = true;
}
+ if (pgcform->relallfrozen != (int32) num_all_frozen_pages)
+ {
+ pgcform->relallfrozen = (int32) num_all_frozen_pages;
+ dirty = true;
+ }
/* Apply DDL updates, but not inside an outer transaction (see above) */
diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c
index 5a2aabc921e..69892f19978 100644
--- a/src/backend/statistics/relation_stats.c
+++ b/src/backend/statistics/relation_stats.c
@@ -54,6 +54,10 @@ static bool relation_statistics_update(FunctionCallInfo fcinfo, int elevel);
/*
* Internal function for modifying statistics for a relation.
+ *
+ * Up to four pg_class columns may be updated even though only three relation
+ * statistics may be modified; relallfrozen is always set to -1 when
+ * relallvisible is updated manually.
*/
static bool
relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
@@ -62,9 +66,9 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
Relation crel;
HeapTuple ctup;
Form_pg_class pgcform;
- int replaces[3] = {0};
- Datum values[3] = {0};
- bool nulls[3] = {0};
+ int replaces[4] = {0};
+ Datum values[4] = {0};
+ bool nulls[4] = {0};
int ncols = 0;
TupleDesc tupdesc;
bool result = true;
@@ -154,6 +158,16 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
replaces[ncols] = Anum_pg_class_relallvisible;
values[ncols] = Int32GetDatum(relallvisible);
ncols++;
+
+ /*
+ * If we are modifying relallvisible manually, it is not clear
+ * what relallfrozen value would make sense. Therefore, set it to
+ * -1, or unknown. It will be updated the next time these fields
+ * are updated.
+ */
+ replaces[ncols] = Anum_pg_class_relallfrozen;
+ values[ncols] = Int32GetDatum(-1);
+ ncols++;
}
}
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index c326f687eb4..4bd9e10310c 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1936,6 +1936,7 @@ formrdesc(const char *relationName, Oid relationReltype,
relation->rd_rel->relpages = 0;
relation->rd_rel->reltuples = -1;
relation->rd_rel->relallvisible = 0;
+ relation->rd_rel->relallfrozen = 0;
relation->rd_rel->relkind = RELKIND_RELATION;
relation->rd_rel->relnatts = (int16) natts;
relation->rd_rel->relam = HEAP_TABLE_AM_OID;
@@ -3931,6 +3932,7 @@ RelationSetNewRelfilenumber(Relation relation, char persistence)
classform->relpages = 0; /* it's empty until further notice */
classform->reltuples = -1;
classform->relallvisible = 0;
+ classform->relallfrozen = 0;
}
classform->relfrozenxid = freezeXid;
classform->relminmxid = minmulti;
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index fc1c125d0d0..21dfcb16d27 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202410242
+#define CATALOG_VERSION_NO 202410251
#endif
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index 0fc2c093b0d..b915bef9aa5 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -68,6 +68,9 @@ CATALOG(pg_class,1259,RelationRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83,Relat
/* # of all-visible blocks (not always up-to-date) */
int32 relallvisible BKI_DEFAULT(0);
+ /* # of all-frozen blocks (not always up-to-date; -1 means "unknown") */
+ int32 relallfrozen BKI_DEFAULT(0);
+
/* OID of toast table; 0 if none */
Oid reltoastrelid BKI_DEFAULT(0) BKI_LOOKUP_OPT(pg_class);
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 759f9a87d38..c3fd2919e64 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -329,6 +329,7 @@ extern void vac_update_relstats(Relation relation,
BlockNumber num_pages,
double num_tuples,
BlockNumber num_all_visible_pages,
+ BlockNumber num_all_frozen_pages,
bool hasindex,
TransactionId frozenxid,
MultiXactId minmulti,
--
2.34.1
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Trigger more frequent autovacuums of heavy insert tables
2024-10-24 19:51 Re: Trigger more frequent autovacuums of heavy insert tables Greg Sabino Mullane <[email protected]>
2024-10-25 15:14 ` Re: Trigger more frequent autovacuums of heavy insert tables Melanie Plageman <[email protected]>
@ 2025-01-16 21:43 ` Melanie Plageman <[email protected]>
2025-01-16 22:50 ` Re: Trigger more frequent autovacuums of heavy insert tables Melanie Plageman <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Melanie Plageman @ 2025-01-16 21:43 UTC (permalink / raw)
To: Greg Sabino Mullane <[email protected]>; +Cc: pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Peter Geoghegan <[email protected]>; David Rowley <[email protected]>
On Fri, Oct 25, 2024 at 11:14 AM Melanie Plageman
<[email protected]> wrote:
>
> I've done something similar to this in attached v2.
This needed a rebase. See attached v4.
- Melanie
Attachments:
[application/octet-stream] v4-0001-Add-relallfrozen-to-pg_class.patch (16.0K, ../../CAAKRu_Yep1JCnvrrZuSLjbJi13GtEkO+_NYSYOOpCMSYK3+mAA@mail.gmail.com/2-v4-0001-Add-relallfrozen-to-pg_class.patch)
download | inline diff:
From 2ed8278b4fb44050c4081b021bf59bcc33fd107e Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Thu, 16 Jan 2025 16:19:57 -0500
Subject: [PATCH v4 1/2] Add relallfrozen to pg_class
Add relallfrozen, an estimate of the number of pages marked all-frozen
in the visibility map.
pg_class already has relallvisible, an estimate of the number of pages
in the relation marked all-visible in the visibility map. This is used
primarily for planning.
relallfrozen, however, is useful for estimating the outstanding number
of all-visible but not all-frozen pages in the relation for the purposes
of scheduling manual VACUUMs and tuning vacuum freeze parameters.
In the future, this field could be used to trigger more frequent vacuums
on insert-focused workloads with significant volume of frozen data.
Reviewed-by: Greg Sabino Mullane
---
doc/src/sgml/catalogs.sgml | 20 ++++++++++++++++++++
src/backend/access/heap/vacuumlazy.c | 17 +++++++++++++----
src/backend/catalog/heap.c | 2 ++
src/backend/catalog/index.c | 22 +++++++++++++++++-----
src/backend/commands/analyze.c | 12 ++++++------
src/backend/commands/cluster.c | 5 +++++
src/backend/commands/vacuum.c | 15 ++++++++++++++-
src/backend/statistics/relation_stats.c | 23 ++++++++++++++++++++---
src/backend/utils/cache/relcache.c | 2 ++
src/include/catalog/pg_class.h | 8 ++++++++
src/include/commands/vacuum.h | 1 +
11 files changed, 108 insertions(+), 19 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index d3036c5ba9d..d218d3702e9 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -2064,6 +2064,26 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
</para></entry>
</row>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>relallfrozen</structfield> <type>int4</type>
+ </para>
+ <para>
+ Number of pages that are marked all-frozen in the table's visibility
+ map. This is only an estimate used for triggering autovacuums. It is
+ updated by <link linkend="sql-vacuum"><command>VACUUM</command></link>,
+ <link linkend="sql-analyze"><command>ANALYZE</command></link>,
+ and a few DDL commands such as
+ <link linkend="sql-createindex"><command>CREATE INDEX</command></link>.
+ <structfield>relallfrozen</structfield> must be less than or equal to
+ <strutfield>relallvisible</structfield> as an all-frozen page must be
+ all-visible. If <structfield>relallvisible</structfield> was updated
+ manually, <structfield>relallfrozen</structfield> will be -1 until the
+ next time they are updated.
+ </para></entry>
+ </row>
+
+
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>reltoastrelid</structfield> <type>oid</type>
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 5b0e790e121..ec73471c2c4 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -370,7 +370,8 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
minmulti_updated;
BlockNumber orig_rel_pages,
new_rel_pages,
- new_rel_allvisible;
+ new_rel_allvisible,
+ new_rel_allfrozen;
PGRUsage ru0;
TimestampTz starttime = 0;
PgStat_Counter startreadtime = 0,
@@ -629,10 +630,17 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
* pg_class.relpages to
*/
new_rel_pages = vacrel->rel_pages; /* After possible rel truncation */
- visibilitymap_count(rel, &new_rel_allvisible, NULL);
+ visibilitymap_count(rel, &new_rel_allvisible, &new_rel_allfrozen);
if (new_rel_allvisible > new_rel_pages)
new_rel_allvisible = new_rel_pages;
+ /*
+ * An all-frozen block _must_ be all-visible. As such, clamp the count of
+ * all-frozen blocks to the count of all-visible blocks.
+ */
+ if (new_rel_allfrozen > new_rel_allvisible)
+ new_rel_allfrozen = new_rel_allvisible;
+
/*
* Now actually update rel's pg_class entry.
*
@@ -641,7 +649,8 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
* scan every page that isn't skipped using the visibility map.
*/
vac_update_relstats(rel, new_rel_pages, vacrel->new_live_tuples,
- new_rel_allvisible, vacrel->nindexes > 0,
+ new_rel_allvisible, new_rel_allfrozen,
+ vacrel->nindexes > 0,
vacrel->NewRelfrozenXid, vacrel->NewRelminMxid,
&frozenxid_updated, &minmulti_updated, false);
@@ -3248,7 +3257,7 @@ update_relstats_all_indexes(LVRelState *vacrel)
vac_update_relstats(indrel,
istat->num_pages,
istat->num_index_tuples,
- 0,
+ 0, 0,
false,
InvalidTransactionId,
InvalidMultiXactId,
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 57ef466acce..5ab75d5c977 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -913,6 +913,7 @@ InsertPgClassTuple(Relation pg_class_desc,
values[Anum_pg_class_relpages - 1] = Int32GetDatum(rd_rel->relpages);
values[Anum_pg_class_reltuples - 1] = Float4GetDatum(rd_rel->reltuples);
values[Anum_pg_class_relallvisible - 1] = Int32GetDatum(rd_rel->relallvisible);
+ values[Anum_pg_class_relallfrozen - 1] = Int32GetDatum(rd_rel->relallfrozen);
values[Anum_pg_class_reltoastrelid - 1] = ObjectIdGetDatum(rd_rel->reltoastrelid);
values[Anum_pg_class_relhasindex - 1] = BoolGetDatum(rd_rel->relhasindex);
values[Anum_pg_class_relisshared - 1] = BoolGetDatum(rd_rel->relisshared);
@@ -983,6 +984,7 @@ AddNewRelationTuple(Relation pg_class_desc,
new_rel_reltup->relpages = 0;
new_rel_reltup->reltuples = -1;
new_rel_reltup->relallvisible = 0;
+ new_rel_reltup->relallfrozen = 0;
/* Sequences always have a known size */
if (relkind == RELKIND_SEQUENCE)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 7377912b41e..ee72a1e5e41 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2792,8 +2792,8 @@ FormIndexDatum(IndexInfo *indexInfo,
* hasindex: set relhasindex to this value
* reltuples: if >= 0, set reltuples to this value; else no change
*
- * If reltuples >= 0, relpages and relallvisible are also updated (using
- * RelationGetNumberOfBlocks() and visibilitymap_count()).
+ * If reltuples >= 0, relpages, relallvisible, and relallfrozen are also
+ * updated (using RelationGetNumberOfBlocks() and visibilitymap_count()).
*
* NOTE: an important side-effect of this operation is that an SI invalidation
* message is sent out to all backends --- including me --- causing relcache
@@ -2811,6 +2811,7 @@ index_update_stats(Relation rel,
bool update_stats;
BlockNumber relpages = 0; /* keep compiler quiet */
BlockNumber relallvisible = 0;
+ BlockNumber relallfrozen = 0;
Oid relid = RelationGetRelid(rel);
Relation pg_class;
ScanKeyData key[1];
@@ -2850,7 +2851,13 @@ index_update_stats(Relation rel,
relpages = RelationGetNumberOfBlocks(rel);
if (rel->rd_rel->relkind != RELKIND_INDEX)
- visibilitymap_count(rel, &relallvisible, NULL);
+ {
+ visibilitymap_count(rel, &relallvisible, &relallfrozen);
+
+ /* An all-frozen block must be all-visible in the VM */
+ if (relallfrozen > relallvisible)
+ relallfrozen = relallvisible;
+ }
}
/*
@@ -2875,8 +2882,8 @@ index_update_stats(Relation rel,
* transaction could still fail before committing. Setting relhasindex
* true is safe even if there are no indexes (VACUUM will eventually fix
* it). And of course the new relpages and reltuples counts are correct
- * regardless. However, we don't want to change relpages (or
- * relallvisible) if the caller isn't providing an updated reltuples
+ * regardless. However, we don't want to change relpages (or relallvisible
+ * and relallfrozen) if the caller isn't providing an updated reltuples
* count, because that would bollix the reltuples/relpages ratio which is
* what's really important.
*/
@@ -2923,6 +2930,11 @@ index_update_stats(Relation rel,
rd_rel->relallvisible = (int32) relallvisible;
dirty = true;
}
+ if (rd_rel->relallfrozen != (int32) relallfrozen)
+ {
+ rd_rel->relallfrozen = (int32) relallfrozen;
+ dirty = true;
+ }
}
/*
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 2a7769b1fd1..927b35ead34 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -628,12 +628,11 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
*/
if (!inh)
{
- BlockNumber relallvisible;
+ BlockNumber relallvisible = 0;
+ BlockNumber relallfrozen = 0;
if (RELKIND_HAS_STORAGE(onerel->rd_rel->relkind))
- visibilitymap_count(onerel, &relallvisible, NULL);
- else
- relallvisible = 0;
+ visibilitymap_count(onerel, &relallvisible, &relallfrozen);
/*
* Update pg_class for table relation. CCI first, in case acquirefunc
@@ -644,6 +643,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
relpages,
totalrows,
relallvisible,
+ relallfrozen,
hasindex,
InvalidTransactionId,
InvalidMultiXactId,
@@ -660,7 +660,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
vac_update_relstats(Irel[ind],
RelationGetNumberOfBlocks(Irel[ind]),
totalindexrows,
- 0,
+ 0, 0,
false,
InvalidTransactionId,
InvalidMultiXactId,
@@ -676,7 +676,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
*/
CommandCounterIncrement();
vac_update_relstats(onerel, -1, totalrows,
- 0, hasindex, InvalidTransactionId,
+ 0, 0, hasindex, InvalidTransactionId,
InvalidMultiXactId,
NULL, NULL,
in_outer_xact);
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 99193f5c886..54a08e4102e 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1226,6 +1226,7 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
int32 swap_pages;
float4 swap_tuples;
int32 swap_allvisible;
+ int32 swap_allfrozen;
swap_pages = relform1->relpages;
relform1->relpages = relform2->relpages;
@@ -1238,6 +1239,10 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
swap_allvisible = relform1->relallvisible;
relform1->relallvisible = relform2->relallvisible;
relform2->relallvisible = swap_allvisible;
+
+ swap_allfrozen = relform1->relallfrozen;
+ relform1->relallfrozen = relform2->relallfrozen;
+ relform2->relallfrozen = swap_allfrozen;
}
/*
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index e6745e6145c..376ce2e489a 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1410,6 +1410,7 @@ void
vac_update_relstats(Relation relation,
BlockNumber num_pages, double num_tuples,
BlockNumber num_all_visible_pages,
+ BlockNumber num_all_frozen_pages,
bool hasindex, TransactionId frozenxid,
MultiXactId minmulti,
bool *frozenxid_updated, bool *minmulti_updated,
@@ -1441,8 +1442,15 @@ vac_update_relstats(Relation relation,
relid);
pgcform = (Form_pg_class) GETSTRUCT(ctup);
- /* Apply statistical updates, if any, to copied tuple */
+ /*
+ * An all-frozen block must be marked all-visible in the VM. While callers
+ * are likely to check this themselves, it is worth ensuring we don't put
+ * incorrect information in pg_class.
+ */
+ if (num_all_frozen_pages > num_all_visible_pages)
+ num_all_frozen_pages = num_all_visible_pages;
+ /* Apply statistical updates, if any, to copied tuple */
dirty = false;
if (pgcform->relpages != (int32) num_pages)
{
@@ -1459,6 +1467,11 @@ vac_update_relstats(Relation relation,
pgcform->relallvisible = (int32) num_all_visible_pages;
dirty = true;
}
+ if (pgcform->relallfrozen != (int32) num_all_frozen_pages)
+ {
+ pgcform->relallfrozen = (int32) num_all_frozen_pages;
+ dirty = true;
+ }
/* Apply DDL updates, but not inside an outer transaction (see above) */
diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c
index 046661d7c3f..758a56f8a46 100644
--- a/src/backend/statistics/relation_stats.c
+++ b/src/backend/statistics/relation_stats.c
@@ -56,6 +56,10 @@ static bool relation_statistics_update(FunctionCallInfo fcinfo, int elevel,
/*
* Internal function for modifying statistics for a relation.
+ *
+ * Up to four pg_class columns may be updated even though only three relation
+ * statistics may be modified; relallfrozen is always set to -1 when
+ * relallvisible is updated manually.
*/
static bool
relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
@@ -167,6 +171,14 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
if (update_relallvisible && pgcform->relallvisible != relallvisible)
{
pgcform->relallvisible = relallvisible;
+
+ /*
+ * If we are modifying relallvisible manually, it is not clear
+ * what relallfrozen value would make sense. Therefore, set it to
+ * -1 ("unknown"). It will be updated the next time these fields
+ * are updated.
+ */
+ pgcform->relallfrozen = -1;
dirty = true;
}
@@ -182,9 +194,9 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
TupleDesc tupdesc = RelationGetDescr(crel);
HeapTuple ctup;
Form_pg_class pgcform;
- int replaces[3] = {0};
- Datum values[3] = {0};
- bool nulls[3] = {0};
+ int replaces[4] = {0};
+ Datum values[4] = {0};
+ bool nulls[4] = {0};
int nreplaces = 0;
ctup = SearchSysCache1(RELOID, ObjectIdGetDatum(reloid));
@@ -217,6 +229,11 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
replaces[nreplaces] = Anum_pg_class_relallvisible;
values[nreplaces] = Int32GetDatum(relallvisible);
nreplaces++;
+
+ /* See comment above in-place update of relallfrozen */
+ replaces[nreplaces] = Anum_pg_class_relallfrozen;
+ values[nreplaces] = Int32GetDatum(-1);
+ nreplaces++;
}
if (nreplaces > 0)
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 43219a9629c..efd6bcb3860 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1925,6 +1925,7 @@ formrdesc(const char *relationName, Oid relationReltype,
relation->rd_rel->relpages = 0;
relation->rd_rel->reltuples = -1;
relation->rd_rel->relallvisible = 0;
+ relation->rd_rel->relallfrozen = 0;
relation->rd_rel->relkind = RELKIND_RELATION;
relation->rd_rel->relnatts = (int16) natts;
@@ -3882,6 +3883,7 @@ RelationSetNewRelfilenumber(Relation relation, char persistence)
classform->relpages = 0; /* it's empty until further notice */
classform->reltuples = -1;
classform->relallvisible = 0;
+ classform->relallfrozen = 0;
}
classform->relfrozenxid = freezeXid;
classform->relminmxid = minmulti;
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index f0d612ca487..fbf42b0e195 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -68,6 +68,14 @@ CATALOG(pg_class,1259,RelationRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83,Relat
/* # of all-visible blocks (not always up-to-date) */
int32 relallvisible BKI_DEFAULT(0);
+ /*
+ * # of all-frozen blocks (not always up-to-date)
+ * Starts as 0 (if it has never been updated).
+ * If relallvisible is manually updated, relallfrozen will be
+ * -1, meaning "unknown"
+ */
+ int32 relallfrozen BKI_DEFAULT(0);
+
/* OID of toast table; 0 if none */
Oid reltoastrelid BKI_DEFAULT(0) BKI_LOOKUP_OPT(pg_class);
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 12d0b61950d..b2a678df09e 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -329,6 +329,7 @@ extern void vac_update_relstats(Relation relation,
BlockNumber num_pages,
double num_tuples,
BlockNumber num_all_visible_pages,
+ BlockNumber num_all_frozen_pages,
bool hasindex,
TransactionId frozenxid,
MultiXactId minmulti,
--
2.45.2
[application/octet-stream] v4-0002-Trigger-more-frequent-autovacuums-with-relallfroz.patch (5.2K, ../../CAAKRu_Yep1JCnvrrZuSLjbJi13GtEkO+_NYSYOOpCMSYK3+mAA@mail.gmail.com/3-v4-0002-Trigger-more-frequent-autovacuums-with-relallfroz.patch)
download | inline diff:
From 3a3ca54a118879d13c18fc2b266ee88edcc338a8 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Thu, 16 Jan 2025 16:31:55 -0500
Subject: [PATCH v4 2/2] Trigger more frequent autovacuums with relallfrozen
Calculate the insert threshold for triggering an autovacuum of a
relation based on the number of unfrozen pages. By only considering the
"active" (unfrozen) portion of the table when calculating how many
tuples to add to the insert threshold, we can trigger more frequent
vacuums of insert-heavy tables and increase the chances of vacuuming
those pages when they still reside in shared buffers.
Reviewed-by: Greg Sabino Mullane
---
doc/src/sgml/config.sgml | 16 +++++-----
src/backend/postmaster/autovacuum.c | 32 +++++++++++++++++--
src/backend/utils/misc/postgresql.conf.sample | 4 +--
3 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a8866292d46..84f2b44ec97 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8651,14 +8651,14 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
</term>
<listitem>
<para>
- Specifies a fraction of the table size to add to
- <varname>autovacuum_vacuum_insert_threshold</varname>
- when deciding whether to trigger a <command>VACUUM</command>.
- The default is <literal>0.2</literal> (20% of table size).
- This parameter can only be set in the <filename>postgresql.conf</filename>
- file or on the server command line;
- but the setting can be overridden for individual tables by
- changing table storage parameters.
+ Specifies a fraction of the active (unfrozen) table size to add to
+ <varname>autovacuum_vacuum_insert_threshold</varname>
+ when deciding whether to trigger a <command>VACUUM</command>.
+ The default is 0.2 (20% of active table size).
+ This parameter can only be set in the <filename>postgresql.conf</filename>
+ file or on the server command line;
+ but the setting can be overridden for individual tables by
+ changing table storage parameters.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 0ab921a169b..2d4a7778561 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2929,7 +2929,6 @@ relation_needs_vacanalyze(Oid relid,
{
bool force_vacuum;
bool av_enabled;
- float4 reltuples; /* pg_class.reltuples */
/* constants from reloptions or GUC variables */
int vac_base_thresh,
@@ -3037,7 +3036,12 @@ relation_needs_vacanalyze(Oid relid,
*/
if (PointerIsValid(tabentry) && AutoVacuumingActive())
{
- reltuples = classForm->reltuples;
+ float4 pcnt_unfrozen = 1;
+ float4 reltuples = classForm->reltuples;
+ int32 relpages = classForm->relpages;
+ int32 relallfrozen = classForm->relallfrozen;
+ int32 relallvisible = classForm->relallvisible;
+
vactuples = tabentry->dead_tuples;
instuples = tabentry->ins_since_vacuum;
anltuples = tabentry->mod_since_analyze;
@@ -3046,8 +3050,30 @@ relation_needs_vacanalyze(Oid relid,
if (reltuples < 0)
reltuples = 0;
+ /*
+ * If the table has been vacuumed and we have reliable data for
+ * relallfrozen and relallvisible, calculate the unfrozen percentage
+ * of the table to modify insert scale factor. This helps us decide
+ * whether or not to vacuum an insert-heavy table based on the number
+ * of inserts to the "active" part of the table.
+ *
+ * If relallfrozen is -1, that means relallvisible was updated
+ * manually and we can't rely on relallfrozen.
+ */
+ if (relpages > 0 && reltuples > 0 && relallfrozen > -1)
+ {
+ if (relallvisible > relpages)
+ relallvisible = relpages;
+
+ if (relallfrozen > relallvisible)
+ relallfrozen = relallvisible;
+
+ pcnt_unfrozen = 1 - ((float4) relallfrozen / relpages);
+ }
+
vacthresh = (float4) vac_base_thresh + vac_scale_factor * reltuples;
- vacinsthresh = (float4) vac_ins_base_thresh + vac_ins_scale_factor * reltuples;
+ vacinsthresh = (float4) vac_ins_base_thresh +
+ vac_ins_scale_factor * reltuples * pcnt_unfrozen;
anlthresh = (float4) anl_base_thresh + anl_scale_factor * reltuples;
/*
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 079efa1baa7..1b10f53c659 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -667,8 +667,8 @@ autovacuum_worker_slots = 16 # autovacuum worker slots to allocate
#autovacuum_analyze_threshold = 50 # min number of row updates before
# analyze
#autovacuum_vacuum_scale_factor = 0.2 # fraction of table size before vacuum
-#autovacuum_vacuum_insert_scale_factor = 0.2 # fraction of inserts over table
- # size before insert vacuum
+#autovacuum_vacuum_insert_scale_factor = 0.2 # fraction of inserts over active
+ # table size before insert vacuum
#autovacuum_analyze_scale_factor = 0.1 # fraction of table size before analyze
#autovacuum_freeze_max_age = 200000000 # maximum XID age before forced vacuum
# (change requires restart)
--
2.45.2
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Trigger more frequent autovacuums of heavy insert tables
2024-10-24 19:51 Re: Trigger more frequent autovacuums of heavy insert tables Greg Sabino Mullane <[email protected]>
2024-10-25 15:14 ` Re: Trigger more frequent autovacuums of heavy insert tables Melanie Plageman <[email protected]>
2025-01-16 21:43 ` Re: Trigger more frequent autovacuums of heavy insert tables Melanie Plageman <[email protected]>
@ 2025-01-16 22:50 ` Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2025-01-16 22:50 UTC (permalink / raw)
To: Greg Sabino Mullane <[email protected]>; +Cc: pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Peter Geoghegan <[email protected]>; David Rowley <[email protected]>
On Thu, Jan 16, 2025 at 4:43 PM Melanie Plageman
<[email protected]> wrote:
>
> On Fri, Oct 25, 2024 at 11:14 AM Melanie Plageman
> <[email protected]> wrote:
> >
> > I've done something similar to this in attached v2.
>
> This needed a rebase. See attached v4.
Whoops -- docs didn't build. Attached v5.
- Melanie
Attachments:
[application/octet-stream] v5-0002-Trigger-more-frequent-autovacuums-with-relallfroz.patch (5.2K, ../../CAAKRu_becyw4kfZDKKWKyYpggtbMx-+TrBdN6pLmRRVKqTzJbw@mail.gmail.com/2-v5-0002-Trigger-more-frequent-autovacuums-with-relallfroz.patch)
download | inline diff:
From c27e370df14f6f20d21a76c9563d6f00019949e1 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Thu, 16 Jan 2025 16:31:55 -0500
Subject: [PATCH v5 2/2] Trigger more frequent autovacuums with relallfrozen
Calculate the insert threshold for triggering an autovacuum of a
relation based on the number of unfrozen pages. By only considering the
"active" (unfrozen) portion of the table when calculating how many
tuples to add to the insert threshold, we can trigger more frequent
vacuums of insert-heavy tables and increase the chances of vacuuming
those pages when they still reside in shared buffers.
Reviewed-by: Greg Sabino Mullane
---
doc/src/sgml/config.sgml | 16 +++++-----
src/backend/postmaster/autovacuum.c | 32 +++++++++++++++++--
src/backend/utils/misc/postgresql.conf.sample | 4 +--
3 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a8866292d46..84f2b44ec97 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8651,14 +8651,14 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
</term>
<listitem>
<para>
- Specifies a fraction of the table size to add to
- <varname>autovacuum_vacuum_insert_threshold</varname>
- when deciding whether to trigger a <command>VACUUM</command>.
- The default is <literal>0.2</literal> (20% of table size).
- This parameter can only be set in the <filename>postgresql.conf</filename>
- file or on the server command line;
- but the setting can be overridden for individual tables by
- changing table storage parameters.
+ Specifies a fraction of the active (unfrozen) table size to add to
+ <varname>autovacuum_vacuum_insert_threshold</varname>
+ when deciding whether to trigger a <command>VACUUM</command>.
+ The default is 0.2 (20% of active table size).
+ This parameter can only be set in the <filename>postgresql.conf</filename>
+ file or on the server command line;
+ but the setting can be overridden for individual tables by
+ changing table storage parameters.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 0ab921a169b..2d4a7778561 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2929,7 +2929,6 @@ relation_needs_vacanalyze(Oid relid,
{
bool force_vacuum;
bool av_enabled;
- float4 reltuples; /* pg_class.reltuples */
/* constants from reloptions or GUC variables */
int vac_base_thresh,
@@ -3037,7 +3036,12 @@ relation_needs_vacanalyze(Oid relid,
*/
if (PointerIsValid(tabentry) && AutoVacuumingActive())
{
- reltuples = classForm->reltuples;
+ float4 pcnt_unfrozen = 1;
+ float4 reltuples = classForm->reltuples;
+ int32 relpages = classForm->relpages;
+ int32 relallfrozen = classForm->relallfrozen;
+ int32 relallvisible = classForm->relallvisible;
+
vactuples = tabentry->dead_tuples;
instuples = tabentry->ins_since_vacuum;
anltuples = tabentry->mod_since_analyze;
@@ -3046,8 +3050,30 @@ relation_needs_vacanalyze(Oid relid,
if (reltuples < 0)
reltuples = 0;
+ /*
+ * If the table has been vacuumed and we have reliable data for
+ * relallfrozen and relallvisible, calculate the unfrozen percentage
+ * of the table to modify insert scale factor. This helps us decide
+ * whether or not to vacuum an insert-heavy table based on the number
+ * of inserts to the "active" part of the table.
+ *
+ * If relallfrozen is -1, that means relallvisible was updated
+ * manually and we can't rely on relallfrozen.
+ */
+ if (relpages > 0 && reltuples > 0 && relallfrozen > -1)
+ {
+ if (relallvisible > relpages)
+ relallvisible = relpages;
+
+ if (relallfrozen > relallvisible)
+ relallfrozen = relallvisible;
+
+ pcnt_unfrozen = 1 - ((float4) relallfrozen / relpages);
+ }
+
vacthresh = (float4) vac_base_thresh + vac_scale_factor * reltuples;
- vacinsthresh = (float4) vac_ins_base_thresh + vac_ins_scale_factor * reltuples;
+ vacinsthresh = (float4) vac_ins_base_thresh +
+ vac_ins_scale_factor * reltuples * pcnt_unfrozen;
anlthresh = (float4) anl_base_thresh + anl_scale_factor * reltuples;
/*
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 079efa1baa7..1b10f53c659 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -667,8 +667,8 @@ autovacuum_worker_slots = 16 # autovacuum worker slots to allocate
#autovacuum_analyze_threshold = 50 # min number of row updates before
# analyze
#autovacuum_vacuum_scale_factor = 0.2 # fraction of table size before vacuum
-#autovacuum_vacuum_insert_scale_factor = 0.2 # fraction of inserts over table
- # size before insert vacuum
+#autovacuum_vacuum_insert_scale_factor = 0.2 # fraction of inserts over active
+ # table size before insert vacuum
#autovacuum_analyze_scale_factor = 0.1 # fraction of table size before analyze
#autovacuum_freeze_max_age = 200000000 # maximum XID age before forced vacuum
# (change requires restart)
--
2.45.2
[application/octet-stream] v5-0001-Add-relallfrozen-to-pg_class.patch (16.0K, ../../CAAKRu_becyw4kfZDKKWKyYpggtbMx-+TrBdN6pLmRRVKqTzJbw@mail.gmail.com/3-v5-0001-Add-relallfrozen-to-pg_class.patch)
download | inline diff:
From 61e40cbe7c9df615461174301984552e31006a09 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Thu, 16 Jan 2025 16:19:57 -0500
Subject: [PATCH v5 1/2] Add relallfrozen to pg_class
Add relallfrozen, an estimate of the number of pages marked all-frozen
in the visibility map.
pg_class already has relallvisible, an estimate of the number of pages
in the relation marked all-visible in the visibility map. This is used
primarily for planning.
relallfrozen, however, is useful for estimating the outstanding number
of all-visible but not all-frozen pages in the relation for the purposes
of scheduling manual VACUUMs and tuning vacuum freeze parameters.
In the future, this field could be used to trigger more frequent vacuums
on insert-focused workloads with significant volume of frozen data.
Reviewed-by: Greg Sabino Mullane
---
doc/src/sgml/catalogs.sgml | 20 ++++++++++++++++++++
src/backend/access/heap/vacuumlazy.c | 17 +++++++++++++----
src/backend/catalog/heap.c | 2 ++
src/backend/catalog/index.c | 22 +++++++++++++++++-----
src/backend/commands/analyze.c | 12 ++++++------
src/backend/commands/cluster.c | 5 +++++
src/backend/commands/vacuum.c | 15 ++++++++++++++-
src/backend/statistics/relation_stats.c | 23 ++++++++++++++++++++---
src/backend/utils/cache/relcache.c | 2 ++
src/include/catalog/pg_class.h | 8 ++++++++
src/include/commands/vacuum.h | 1 +
11 files changed, 108 insertions(+), 19 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index d3036c5ba9d..bc9a735e5a7 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -2064,6 +2064,26 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
</para></entry>
</row>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>relallfrozen</structfield> <type>int4</type>
+ </para>
+ <para>
+ Number of pages that are marked all-frozen in the table's visibility
+ map. This is only an estimate used for triggering autovacuums. It is
+ updated by <link linkend="sql-vacuum"><command>VACUUM</command></link>,
+ <link linkend="sql-analyze"><command>ANALYZE</command></link>,
+ and a few DDL commands such as
+ <link linkend="sql-createindex"><command>CREATE INDEX</command></link>.
+ <structfield>relallfrozen</structfield> must be less than or equal to
+ <structfield>relallvisible</structfield> as an all-frozen page must be
+ all-visible. If <structfield>relallvisible</structfield> was updated
+ manually, <structfield>relallfrozen</structfield> will be -1 until the
+ next time they are updated.
+ </para></entry>
+ </row>
+
+
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>reltoastrelid</structfield> <type>oid</type>
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 5b0e790e121..ec73471c2c4 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -370,7 +370,8 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
minmulti_updated;
BlockNumber orig_rel_pages,
new_rel_pages,
- new_rel_allvisible;
+ new_rel_allvisible,
+ new_rel_allfrozen;
PGRUsage ru0;
TimestampTz starttime = 0;
PgStat_Counter startreadtime = 0,
@@ -629,10 +630,17 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
* pg_class.relpages to
*/
new_rel_pages = vacrel->rel_pages; /* After possible rel truncation */
- visibilitymap_count(rel, &new_rel_allvisible, NULL);
+ visibilitymap_count(rel, &new_rel_allvisible, &new_rel_allfrozen);
if (new_rel_allvisible > new_rel_pages)
new_rel_allvisible = new_rel_pages;
+ /*
+ * An all-frozen block _must_ be all-visible. As such, clamp the count of
+ * all-frozen blocks to the count of all-visible blocks.
+ */
+ if (new_rel_allfrozen > new_rel_allvisible)
+ new_rel_allfrozen = new_rel_allvisible;
+
/*
* Now actually update rel's pg_class entry.
*
@@ -641,7 +649,8 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
* scan every page that isn't skipped using the visibility map.
*/
vac_update_relstats(rel, new_rel_pages, vacrel->new_live_tuples,
- new_rel_allvisible, vacrel->nindexes > 0,
+ new_rel_allvisible, new_rel_allfrozen,
+ vacrel->nindexes > 0,
vacrel->NewRelfrozenXid, vacrel->NewRelminMxid,
&frozenxid_updated, &minmulti_updated, false);
@@ -3248,7 +3257,7 @@ update_relstats_all_indexes(LVRelState *vacrel)
vac_update_relstats(indrel,
istat->num_pages,
istat->num_index_tuples,
- 0,
+ 0, 0,
false,
InvalidTransactionId,
InvalidMultiXactId,
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 57ef466acce..5ab75d5c977 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -913,6 +913,7 @@ InsertPgClassTuple(Relation pg_class_desc,
values[Anum_pg_class_relpages - 1] = Int32GetDatum(rd_rel->relpages);
values[Anum_pg_class_reltuples - 1] = Float4GetDatum(rd_rel->reltuples);
values[Anum_pg_class_relallvisible - 1] = Int32GetDatum(rd_rel->relallvisible);
+ values[Anum_pg_class_relallfrozen - 1] = Int32GetDatum(rd_rel->relallfrozen);
values[Anum_pg_class_reltoastrelid - 1] = ObjectIdGetDatum(rd_rel->reltoastrelid);
values[Anum_pg_class_relhasindex - 1] = BoolGetDatum(rd_rel->relhasindex);
values[Anum_pg_class_relisshared - 1] = BoolGetDatum(rd_rel->relisshared);
@@ -983,6 +984,7 @@ AddNewRelationTuple(Relation pg_class_desc,
new_rel_reltup->relpages = 0;
new_rel_reltup->reltuples = -1;
new_rel_reltup->relallvisible = 0;
+ new_rel_reltup->relallfrozen = 0;
/* Sequences always have a known size */
if (relkind == RELKIND_SEQUENCE)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 7377912b41e..ee72a1e5e41 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2792,8 +2792,8 @@ FormIndexDatum(IndexInfo *indexInfo,
* hasindex: set relhasindex to this value
* reltuples: if >= 0, set reltuples to this value; else no change
*
- * If reltuples >= 0, relpages and relallvisible are also updated (using
- * RelationGetNumberOfBlocks() and visibilitymap_count()).
+ * If reltuples >= 0, relpages, relallvisible, and relallfrozen are also
+ * updated (using RelationGetNumberOfBlocks() and visibilitymap_count()).
*
* NOTE: an important side-effect of this operation is that an SI invalidation
* message is sent out to all backends --- including me --- causing relcache
@@ -2811,6 +2811,7 @@ index_update_stats(Relation rel,
bool update_stats;
BlockNumber relpages = 0; /* keep compiler quiet */
BlockNumber relallvisible = 0;
+ BlockNumber relallfrozen = 0;
Oid relid = RelationGetRelid(rel);
Relation pg_class;
ScanKeyData key[1];
@@ -2850,7 +2851,13 @@ index_update_stats(Relation rel,
relpages = RelationGetNumberOfBlocks(rel);
if (rel->rd_rel->relkind != RELKIND_INDEX)
- visibilitymap_count(rel, &relallvisible, NULL);
+ {
+ visibilitymap_count(rel, &relallvisible, &relallfrozen);
+
+ /* An all-frozen block must be all-visible in the VM */
+ if (relallfrozen > relallvisible)
+ relallfrozen = relallvisible;
+ }
}
/*
@@ -2875,8 +2882,8 @@ index_update_stats(Relation rel,
* transaction could still fail before committing. Setting relhasindex
* true is safe even if there are no indexes (VACUUM will eventually fix
* it). And of course the new relpages and reltuples counts are correct
- * regardless. However, we don't want to change relpages (or
- * relallvisible) if the caller isn't providing an updated reltuples
+ * regardless. However, we don't want to change relpages (or relallvisible
+ * and relallfrozen) if the caller isn't providing an updated reltuples
* count, because that would bollix the reltuples/relpages ratio which is
* what's really important.
*/
@@ -2923,6 +2930,11 @@ index_update_stats(Relation rel,
rd_rel->relallvisible = (int32) relallvisible;
dirty = true;
}
+ if (rd_rel->relallfrozen != (int32) relallfrozen)
+ {
+ rd_rel->relallfrozen = (int32) relallfrozen;
+ dirty = true;
+ }
}
/*
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 2a7769b1fd1..927b35ead34 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -628,12 +628,11 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
*/
if (!inh)
{
- BlockNumber relallvisible;
+ BlockNumber relallvisible = 0;
+ BlockNumber relallfrozen = 0;
if (RELKIND_HAS_STORAGE(onerel->rd_rel->relkind))
- visibilitymap_count(onerel, &relallvisible, NULL);
- else
- relallvisible = 0;
+ visibilitymap_count(onerel, &relallvisible, &relallfrozen);
/*
* Update pg_class for table relation. CCI first, in case acquirefunc
@@ -644,6 +643,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
relpages,
totalrows,
relallvisible,
+ relallfrozen,
hasindex,
InvalidTransactionId,
InvalidMultiXactId,
@@ -660,7 +660,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
vac_update_relstats(Irel[ind],
RelationGetNumberOfBlocks(Irel[ind]),
totalindexrows,
- 0,
+ 0, 0,
false,
InvalidTransactionId,
InvalidMultiXactId,
@@ -676,7 +676,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
*/
CommandCounterIncrement();
vac_update_relstats(onerel, -1, totalrows,
- 0, hasindex, InvalidTransactionId,
+ 0, 0, hasindex, InvalidTransactionId,
InvalidMultiXactId,
NULL, NULL,
in_outer_xact);
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 99193f5c886..54a08e4102e 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1226,6 +1226,7 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
int32 swap_pages;
float4 swap_tuples;
int32 swap_allvisible;
+ int32 swap_allfrozen;
swap_pages = relform1->relpages;
relform1->relpages = relform2->relpages;
@@ -1238,6 +1239,10 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
swap_allvisible = relform1->relallvisible;
relform1->relallvisible = relform2->relallvisible;
relform2->relallvisible = swap_allvisible;
+
+ swap_allfrozen = relform1->relallfrozen;
+ relform1->relallfrozen = relform2->relallfrozen;
+ relform2->relallfrozen = swap_allfrozen;
}
/*
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index e6745e6145c..376ce2e489a 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1410,6 +1410,7 @@ void
vac_update_relstats(Relation relation,
BlockNumber num_pages, double num_tuples,
BlockNumber num_all_visible_pages,
+ BlockNumber num_all_frozen_pages,
bool hasindex, TransactionId frozenxid,
MultiXactId minmulti,
bool *frozenxid_updated, bool *minmulti_updated,
@@ -1441,8 +1442,15 @@ vac_update_relstats(Relation relation,
relid);
pgcform = (Form_pg_class) GETSTRUCT(ctup);
- /* Apply statistical updates, if any, to copied tuple */
+ /*
+ * An all-frozen block must be marked all-visible in the VM. While callers
+ * are likely to check this themselves, it is worth ensuring we don't put
+ * incorrect information in pg_class.
+ */
+ if (num_all_frozen_pages > num_all_visible_pages)
+ num_all_frozen_pages = num_all_visible_pages;
+ /* Apply statistical updates, if any, to copied tuple */
dirty = false;
if (pgcform->relpages != (int32) num_pages)
{
@@ -1459,6 +1467,11 @@ vac_update_relstats(Relation relation,
pgcform->relallvisible = (int32) num_all_visible_pages;
dirty = true;
}
+ if (pgcform->relallfrozen != (int32) num_all_frozen_pages)
+ {
+ pgcform->relallfrozen = (int32) num_all_frozen_pages;
+ dirty = true;
+ }
/* Apply DDL updates, but not inside an outer transaction (see above) */
diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c
index 046661d7c3f..758a56f8a46 100644
--- a/src/backend/statistics/relation_stats.c
+++ b/src/backend/statistics/relation_stats.c
@@ -56,6 +56,10 @@ static bool relation_statistics_update(FunctionCallInfo fcinfo, int elevel,
/*
* Internal function for modifying statistics for a relation.
+ *
+ * Up to four pg_class columns may be updated even though only three relation
+ * statistics may be modified; relallfrozen is always set to -1 when
+ * relallvisible is updated manually.
*/
static bool
relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
@@ -167,6 +171,14 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
if (update_relallvisible && pgcform->relallvisible != relallvisible)
{
pgcform->relallvisible = relallvisible;
+
+ /*
+ * If we are modifying relallvisible manually, it is not clear
+ * what relallfrozen value would make sense. Therefore, set it to
+ * -1 ("unknown"). It will be updated the next time these fields
+ * are updated.
+ */
+ pgcform->relallfrozen = -1;
dirty = true;
}
@@ -182,9 +194,9 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
TupleDesc tupdesc = RelationGetDescr(crel);
HeapTuple ctup;
Form_pg_class pgcform;
- int replaces[3] = {0};
- Datum values[3] = {0};
- bool nulls[3] = {0};
+ int replaces[4] = {0};
+ Datum values[4] = {0};
+ bool nulls[4] = {0};
int nreplaces = 0;
ctup = SearchSysCache1(RELOID, ObjectIdGetDatum(reloid));
@@ -217,6 +229,11 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
replaces[nreplaces] = Anum_pg_class_relallvisible;
values[nreplaces] = Int32GetDatum(relallvisible);
nreplaces++;
+
+ /* See comment above in-place update of relallfrozen */
+ replaces[nreplaces] = Anum_pg_class_relallfrozen;
+ values[nreplaces] = Int32GetDatum(-1);
+ nreplaces++;
}
if (nreplaces > 0)
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 43219a9629c..efd6bcb3860 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1925,6 +1925,7 @@ formrdesc(const char *relationName, Oid relationReltype,
relation->rd_rel->relpages = 0;
relation->rd_rel->reltuples = -1;
relation->rd_rel->relallvisible = 0;
+ relation->rd_rel->relallfrozen = 0;
relation->rd_rel->relkind = RELKIND_RELATION;
relation->rd_rel->relnatts = (int16) natts;
@@ -3882,6 +3883,7 @@ RelationSetNewRelfilenumber(Relation relation, char persistence)
classform->relpages = 0; /* it's empty until further notice */
classform->reltuples = -1;
classform->relallvisible = 0;
+ classform->relallfrozen = 0;
}
classform->relfrozenxid = freezeXid;
classform->relminmxid = minmulti;
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index f0d612ca487..fbf42b0e195 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -68,6 +68,14 @@ CATALOG(pg_class,1259,RelationRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83,Relat
/* # of all-visible blocks (not always up-to-date) */
int32 relallvisible BKI_DEFAULT(0);
+ /*
+ * # of all-frozen blocks (not always up-to-date)
+ * Starts as 0 (if it has never been updated).
+ * If relallvisible is manually updated, relallfrozen will be
+ * -1, meaning "unknown"
+ */
+ int32 relallfrozen BKI_DEFAULT(0);
+
/* OID of toast table; 0 if none */
Oid reltoastrelid BKI_DEFAULT(0) BKI_LOOKUP_OPT(pg_class);
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 12d0b61950d..b2a678df09e 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -329,6 +329,7 @@ extern void vac_update_relstats(Relation relation,
BlockNumber num_pages,
double num_tuples,
BlockNumber num_all_visible_pages,
+ BlockNumber num_all_frozen_pages,
bool hasindex,
TransactionId frozenxid,
MultiXactId minmulti,
--
2.45.2
^ permalink raw reply [nested|flat] 28+ messages in thread
end of thread, other threads:[~2025-01-16 22:50 UTC | newest]
Thread overview: 28+ 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]>
2024-10-24 19:51 Re: Trigger more frequent autovacuums of heavy insert tables Greg Sabino Mullane <[email protected]>
2024-10-25 15:14 ` Re: Trigger more frequent autovacuums of heavy insert tables Melanie Plageman <[email protected]>
2025-01-16 21:43 ` Re: Trigger more frequent autovacuums of heavy insert tables Melanie Plageman <[email protected]>
2025-01-16 22:50 ` Re: Trigger more frequent autovacuums of heavy insert tables Melanie Plageman <[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