MIME-Version: 1.0
References: <bee34abe-fe87-49b3-8523-8ae19e641ccf@yandex.ru>
 <ff99283d-f9d4-4b37-b4fa-3a433fb408a4@tantorlabs.com> <e1f5b560691a4fe693625d529e19e13959210568.camel@moonset.ru>
 <53c47c2d-72a5-44f2-900c-9973b2af1808@tantorlabs.com> <c82e7776-7841-42a0-9247-878584174750@tantorlabs.com>
 <4a902cea-54fb-41b5-b208-b84731a5f577@postgrespro.ru>
In-Reply-To: <4a902cea-54fb-41b5-b208-b84731a5f577@postgrespro.ru>
From: jian he <jian.universality@gmail.com>
Date: Fri, 16 Aug 2024 19:12:00 +0800
Message-ID: <CACJufxHb_YGCp=pVH6DZcpk9yML+SueffPeaRbX2LzXZVahd_w@mail.gmail.com>
Subject: Re: Vacuum statistics
To: Alena Rybakina <a.rybakina@postgrespro.ru>
Cc: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>, Andrei Zubkov <zubkov@moonset.ru>, 
	Alena Rybakina <lena.ribackina@yandex.ru>, pgsql-hackers <pgsql-hackers@postgresql.org>, 
	a.lepikhov@postgrespro.ru
Content-Type: multipart/mixed; boundary="00000000000068faff061fcb0829"
Archived-At: <https://www.postgresql.org/message-id/CACJufxHb_YGCp%3DpVH6DZcpk9yML%2BSueffPeaRbX2LzXZVahd_w%40mail.gmail.com>
Precedence: bulk

--00000000000068faff061fcb0829
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Aug 15, 2024 at 4:49=E2=80=AFPM Alena Rybakina
<a.rybakina@postgrespro.ru> wrote:
>
> Hi!


I've applied all the v5 patches.
0002 and 0003 have white space errors.

+      <para>
+        Number of times blocks of this index were already found
+        in the buffer cache by vacuum operations, so that a read was
not necessary
+        (this only includes hits in the
+        &project; buffer cache, not the operating system's file system cac=
he)
+      </para></entry>

+        Number of times blocks of this table were already found
+        in the buffer cache by vacuum operations, so that a read was
not necessary
+        (this only includes hits in the
+        &project; buffer cache, not the operating system's file system cac=
he)
+      </para></entry>

"&project;"
represents a sgml file placeholder name as "project" and puts all the
content of "project.sgml" to system-views.sgml.
but you don't have "project.sgml". you may check
doc/src/sgml/filelist.sgml or doc/src/sgml/ref/allfiles.sgml
for usage of "&place_holder;".
so you can change it to "project", otherwise doc cannot build.


src/backend/commands/dbcommands.c
we have:
    /*
     * If built with appropriate switch, whine when regression-testing
     * conventions for database names are violated.  But don't complain dur=
ing
     * initdb.
     */
#ifdef ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
    if (IsUnderPostmaster && strstr(dbname, "regression") =3D=3D NULL)
        elog(WARNING, "databases created by regression test cases
should have names including \"regression\"");
#endif
so in src/test/regress/sql/vacuum_tables_and_db_statistics.sql you
need to change dbname:
CREATE DATABASE statistic_vacuum_database;
CREATE DATABASE statistic_vacuum_database1;


+  <para>
+   The view <structname>pg_stat_vacuum_indexes</structname> will contain
+   one row for each index in the current database (including TOAST
+   table indexes), showing statistics about vacuuming that specific index.
+  </para>
TOAST should
<acronym>TOAST</acronym>


+ /* Build a tuple descriptor for our result type */
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) !=3D TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
maybe change to
            ereport(ERROR,
                    (errcode(ERRCODE_DATATYPE_MISMATCH),
                     errmsg("return type must be a row type")));
Later I found out "InitMaterializedSRF(fcinfo, 0);" already did all
the work. much of the code can be gotten rid of.
please check attached.


>>>>
#define EXTVACHEAPSTAT_COLUMNS    27
#define EXTVACIDXSTAT_COLUMNS    19
#define EXTVACDBSTAT_COLUMNS    15
#define EXTVACSTAT_COLUMNS Max(EXTVACHEAPSTAT_COLUMNS, EXTVACIDXSTAT_COLUMN=
S)

static Oid CurrentDatabaseId =3D InvalidOid;
>>>>
we already defined MyDatabaseId in src/include/miscadmin.h,
Why do we need "static Oid CurrentDatabaseId =3D InvalidOid;"?
also src/backend/utils/adt/pgstatfuncs.c already included "miscadmin.h".


the following code one function has 2 return statements?
------------------------------------------------------------------------
/*
 * Get the vacuum statistics for the heap tables.
 */
Datum
pg_stat_vacuum_tables(PG_FUNCTION_ARGS)
{
    return pg_stats_vacuum(fcinfo, PGSTAT_EXTVAC_HEAP, EXTVACHEAPSTAT_COLUM=
NS);

    PG_RETURN_NULL();
}

/*
 * Get the vacuum statistics for the indexes.
 */
Datum
pg_stat_vacuum_indexes(PG_FUNCTION_ARGS)
{
    return pg_stats_vacuum(fcinfo, PGSTAT_EXTVAC_INDEX, EXTVACIDXSTAT_COLUM=
NS);

    PG_RETURN_NULL();
}

/*
 * Get the vacuum statistics for the database.
 */
Datum
pg_stat_vacuum_database(PG_FUNCTION_ARGS)
{
        return pg_stats_vacuum(fcinfo, PGSTAT_EXTVAC_DB, EXTVACDBSTAT_COLUM=
NS);

    PG_RETURN_NULL();
}
------------------------------------------------------------------------
in pg_stats_vacuum:
    if (type =3D=3D PGSTAT_EXTVAC_INDEX || type =3D=3D PGSTAT_EXTVAC_HEAP)
    {
        Oid                    relid =3D PG_GETARG_OID(1);

        /* Load table statistics for specified database. */
        if (OidIsValid(relid))
        {
            tabentry =3D fetch_dbstat_tabentry(dbid, relid);
            if (tabentry =3D=3D NULL || tabentry->vacuum_ext.type !=3D type=
)
                /* Table don't exists or isn't an heap relation. */
                PG_RETURN_NULL();

            tuplestore_put_for_relation(relid, tupstore, tupdesc,
tabentry, ncolumns);
        }
        else
        {
         ...
        }
}
I don't understand the ELSE branch. the IF branch means the input
dboid, heap/index oid is correct.
the ELSE branch means table reloid is invalid =3D 0.
I am not sure 100% what the ELSE Branch means.
Also there are no comments explaining why.
experiments seem to show that  when reloid is 0, it will print out all
the vacuum statistics
for all the tables in the current database. If so, then only super
users can call pg_stats_vacuum?
but the table owner should be able to call pg_stats_vacuum for that
specific table.


/* Type of ExtVacReport */
typedef enum ExtVacReportType
{
    PGSTAT_EXTVAC_INVALID =3D 0,
    PGSTAT_EXTVAC_HEAP =3D 1,
    PGSTAT_EXTVAC_INDEX =3D 2,
    PGSTAT_EXTVAC_DB =3D 3,
} ExtVacReportType;
generally "HEAP" means table and index, maybe "PGSTAT_EXTVAC_HEAP" would be=
 term

--00000000000068faff061fcb0829
Content-Type: application/octet-stream; 
	name="v5-0001-minor-refactor-pg_stats_vacuum-and-sub-routine.no-cfbot"
Content-Disposition: attachment; 
	filename="v5-0001-minor-refactor-pg_stats_vacuum-and-sub-routine.no-cfbot"
Content-Transfer-Encoding: base64
Content-ID: <f_lzwjv87r0>
X-Attachment-Id: f_lzwjv87r0

RnJvbSA0N2VmMmE3NDZlNmYxN2E4NmRjNDY4NTY1ZGIzYTgyNWNmMDUyNmZjIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBqaWFuIGhlIDxqaWFuLnVuaXZlcnNhbGl0eUBnbWFpbC5jb20+
CkRhdGU6IEZyaSwgMTYgQXVnIDIwMjQgMTg6MDg6MzkgKzA4MDAKU3ViamVjdDogW1BBVENIIHY1
IDEvMV0gbWlub3IgcmVmYWN0b3IgcGdfc3RhdHNfdmFjdXVtIGFuZCBzdWItcm91dGluZXMKCi0t
LQogc3JjL2JhY2tlbmQvdXRpbHMvYWR0L3Bnc3RhdGZ1bmNzLmMgfCA0NSArKysrKysrKystLS0t
LS0tLS0tLS0tLS0tLS0tLQogMSBmaWxlIGNoYW5nZWQsIDEzIGluc2VydGlvbnMoKyksIDMyIGRl
bGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3V0aWxzL2FkdC9wZ3N0YXRmdW5j
cy5jIGIvc3JjL2JhY2tlbmQvdXRpbHMvYWR0L3Bnc3RhdGZ1bmNzLmMKaW5kZXggMGM0OTBiYTVm
MS4uYWRiMDQ3OTVmZiAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvdXRpbHMvYWR0L3Bnc3RhdGZ1
bmNzLmMKKysrIGIvc3JjL2JhY2tlbmQvdXRpbHMvYWR0L3Bnc3RhdGZ1bmNzLmMKQEAgLTIwNzgs
OCArMjA3OCw4IEBAIGZldGNoX2Ric3RhdF90YWJlbnRyeShPaWQgZGJpZCwgT2lkIHJlbGlkKQog
fQogCiBzdGF0aWMgdm9pZAotdHVwbGVzdG9yZV9wdXRfZm9yX2RhdGFiYXNlKE9pZCBkYmlkLCBU
dXBsZXN0b3Jlc3RhdGUgKnR1cHN0b3JlLAotCQkJICAgVHVwbGVEZXNjIHR1cGRlc2MsIFBnU3Rh
dFNoYXJlZF9EYXRhYmFzZSAqZGJlbnRyeSwgaW50IG5jb2x1bW5zKQordHVwbGVzdG9yZV9wdXRf
Zm9yX2RhdGFiYXNlKE9pZCBkYmlkLCBSZXR1cm5TZXRJbmZvICpyc2luZm8sCisJCQkJCQkJUGdT
dGF0U2hhcmVkX0RhdGFiYXNlICpkYmVudHJ5KQogewogCURhdHVtCQl2YWx1ZXNbRVhUVkFDREJT
VEFUX0NPTFVNTlNdOwogCWJvb2wJCW51bGxzW0VYVFZBQ0RCU1RBVF9DT0xVTU5TXTsKQEAgLTIx
MTMsMTUgKzIxMTMsMTQgQEAgdHVwbGVzdG9yZV9wdXRfZm9yX2RhdGFiYXNlKE9pZCBkYmlkLCBU
dXBsZXN0b3Jlc3RhdGUgKnR1cHN0b3JlLAogCXZhbHVlc1tpKytdID0gRmxvYXQ4R2V0RGF0dW0o
ZGJlbnRyeS0+c3RhdHMudmFjdXVtX2V4dC50b3RhbF90aW1lKTsKIAl2YWx1ZXNbaSsrXSA9IElu
dDMyR2V0RGF0dW0oZGJlbnRyeS0+c3RhdHMudmFjdXVtX2V4dC5pbnRlcnJ1cHRzKTsKIAorCUFz
c2VydChpID09IHJzaW5mby0+c2V0RGVzYy0+bmF0dHMpOwogCi0JQXNzZXJ0KGkgPT0gbmNvbHVt
bnMpOwotCi0JdHVwbGVzdG9yZV9wdXR2YWx1ZXModHVwc3RvcmUsIHR1cGRlc2MsIHZhbHVlcywg
bnVsbHMpOworCXR1cGxlc3RvcmVfcHV0dmFsdWVzKHJzaW5mby0+c2V0UmVzdWx0LCByc2luZm8t
PnNldERlc2MsIHZhbHVlcywgbnVsbHMpOwogfQogCiBzdGF0aWMgdm9pZAotdHVwbGVzdG9yZV9w
dXRfZm9yX3JlbGF0aW9uKE9pZCByZWxpZCwgVHVwbGVzdG9yZXN0YXRlICp0dXBzdG9yZSwKLQkJ
CSAgIFR1cGxlRGVzYyB0dXBkZXNjLCBQZ1N0YXRfU3RhdFRhYkVudHJ5ICp0YWJlbnRyeSwgaW50
IG5jb2x1bW5zKQordHVwbGVzdG9yZV9wdXRfZm9yX3JlbGF0aW9uKE9pZCByZWxpZCwgUmV0dXJu
U2V0SW5mbyAqcnNpbmZvLAorCQkJCQkJCVBnU3RhdF9TdGF0VGFiRW50cnkgKnRhYmVudHJ5KQog
ewogCURhdHVtCQl2YWx1ZXNbRVhUVkFDU1RBVF9DT0xVTU5TXTsKIAlib29sCQludWxsc1tFWFRW
QUNTVEFUX0NPTFVNTlNdOwpAQCAtMjE3OSw5ICsyMTc4LDggQEAgdHVwbGVzdG9yZV9wdXRfZm9y
X3JlbGF0aW9uKE9pZCByZWxpZCwgVHVwbGVzdG9yZXN0YXRlICp0dXBzdG9yZSwKIAl2YWx1ZXNb
aSsrXSA9IEZsb2F0OEdldERhdHVtKHRhYmVudHJ5LT52YWN1dW1fZXh0LnRvdGFsX3RpbWUpOwog
CXZhbHVlc1tpKytdID0gSW50MzJHZXREYXR1bSh0YWJlbnRyeS0+dmFjdXVtX2V4dC5pbnRlcnJ1
cHRzKTsKIAotCUFzc2VydChpID09IG5jb2x1bW5zKTsKLQotCXR1cGxlc3RvcmVfcHV0dmFsdWVz
KHR1cHN0b3JlLCB0dXBkZXNjLCB2YWx1ZXMsIG51bGxzKTsKKwlBc3NlcnQoaSA9PSByc2luZm8t
PnNldERlc2MtPm5hdHRzKTsKKwl0dXBsZXN0b3JlX3B1dHZhbHVlcyhyc2luZm8tPnNldFJlc3Vs
dCwgcnNpbmZvLT5zZXREZXNjLCB2YWx1ZXMsIG51bGxzKTsKIH0KIAogLyoKQEAgLTIxOTEsMTAg
KzIxODksNiBAQCBzdGF0aWMgRGF0dW0KIHBnX3N0YXRzX3ZhY3V1bShGdW5jdGlvbkNhbGxJbmZv
IGZjaW5mbywgRXh0VmFjUmVwb3J0VHlwZSB0eXBlLCBpbnQgbmNvbHVtbnMpCiB7CiAJUmV0dXJu
U2V0SW5mbwkJICAgKnJzaW5mbyA9IChSZXR1cm5TZXRJbmZvICopIGZjaW5mby0+cmVzdWx0aW5m
bzsKLQlNZW1vcnlDb250ZXh0CQkJcGVyX3F1ZXJ5X2N0eDsKLQlNZW1vcnlDb250ZXh0CQkJb2xk
Y29udGV4dDsKLQlUdXBsZXN0b3Jlc3RhdGUJCSAgICp0dXBzdG9yZTsKLQlUdXBsZURlc2MJCQkJ
dHVwZGVzYzsKIAlPaWQJCQkJCQlkYmlkID0gUEdfR0VUQVJHX09JRCgwKTsKIAlQZ1N0YXRfU3Rh
dFRhYkVudHJ5ICAgICp0YWJlbnRyeTsKIApAQCAtMjIwNSwyMiArMjE5OSw5IEBAIHBnX3N0YXRz
X3ZhY3V1bShGdW5jdGlvbkNhbGxJbmZvIGZjaW5mbywgRXh0VmFjUmVwb3J0VHlwZSB0eXBlLCBp
bnQgbmNvbHVtbnMpCiAJCWVyZXBvcnQoRVJST1IsCiAJCQkJKGVycmNvZGUoRVJSQ09ERV9GRUFU
VVJFX05PVF9TVVBQT1JURUQpLAogCQkJCSBlcnJtc2coInNldC12YWx1ZWQgZnVuY3Rpb24gY2Fs
bGVkIGluIGNvbnRleHQgdGhhdCBjYW5ub3QgYWNjZXB0IGEgc2V0IikpKTsKLQkvKiBTd2l0Y2gg
dG8gbG9uZy1saXZlZCBjb250ZXh0IHRvIGNyZWF0ZSB0aGUgcmV0dXJuZWQgZGF0YSBzdHJ1Y3R1
cmVzICovCi0JcGVyX3F1ZXJ5X2N0eCA9IHJzaW5mby0+ZWNvbnRleHQtPmVjeHRfcGVyX3F1ZXJ5
X21lbW9yeTsKLQlvbGRjb250ZXh0ID0gTWVtb3J5Q29udGV4dFN3aXRjaFRvKHBlcl9xdWVyeV9j
dHgpOwogCi0JLyogQnVpbGQgYSB0dXBsZSBkZXNjcmlwdG9yIGZvciBvdXIgcmVzdWx0IHR5cGUg
Ki8KLQlpZiAoZ2V0X2NhbGxfcmVzdWx0X3R5cGUoZmNpbmZvLCBOVUxMLCAmdHVwZGVzYykgIT0g
VFlQRUZVTkNfQ09NUE9TSVRFKQotCQllbG9nKEVSUk9SLCAicmV0dXJuIHR5cGUgbXVzdCBiZSBh
IHJvdyB0eXBlIik7Ci0KLQlBc3NlcnQodHVwZGVzYy0+bmF0dHMgPT0gbmNvbHVtbnMpOwotCi0J
dHVwc3RvcmUgPSB0dXBsZXN0b3JlX2JlZ2luX2hlYXAodHJ1ZSwgZmFsc2UsIHdvcmtfbWVtKTsK
LQlBc3NlcnQgKHR1cHN0b3JlICE9IE5VTEwpOwotCXJzaW5mby0+c2V0UmVzdWx0ID0gdHVwc3Rv
cmU7Ci0JcnNpbmZvLT5zZXREZXNjID0gdHVwZGVzYzsKLQotCU1lbW9yeUNvbnRleHRTd2l0Y2hU
byhvbGRjb250ZXh0KTsKKwlBc3NlcnQocnNpbmZvLT5zZXREZXNjLT5uYXR0cyA9PSBuY29sdW1u
cyk7CisJQXNzZXJ0KHJzaW5mby0+c2V0UmVzdWx0ICE9IE5VTEwpOwogCiAJaWYgKHR5cGUgPT0g
UEdTVEFUX0VYVFZBQ19JTkRFWCB8fCB0eXBlID09IFBHU1RBVF9FWFRWQUNfSEVBUCkKIAl7CkBA
IC0yMjM0LDcgKzIyMTUsNyBAQCBwZ19zdGF0c192YWN1dW0oRnVuY3Rpb25DYWxsSW5mbyBmY2lu
Zm8sIEV4dFZhY1JlcG9ydFR5cGUgdHlwZSwgaW50IG5jb2x1bW5zKQogCQkJCS8qIFRhYmxlIGRv
bid0IGV4aXN0cyBvciBpc24ndCBhbiBoZWFwIHJlbGF0aW9uLiAqLwogCQkJCVBHX1JFVFVSTl9O
VUxMKCk7CiAKLQkJCXR1cGxlc3RvcmVfcHV0X2Zvcl9yZWxhdGlvbihyZWxpZCwgdHVwc3RvcmUs
IHR1cGRlc2MsIHRhYmVudHJ5LCBuY29sdW1ucyk7CisJCQl0dXBsZXN0b3JlX3B1dF9mb3JfcmVs
YXRpb24ocmVsaWQsIHJzaW5mbywgdGFiZW50cnkpOwogCQl9CiAJCWVsc2UKIAkJewpAQCAtMjI1
OSw3ICsyMjQwLDcgQEAgcGdfc3RhdHNfdmFjdXVtKEZ1bmN0aW9uQ2FsbEluZm8gZmNpbmZvLCBF
eHRWYWNSZXBvcnRUeXBlIHR5cGUsIGludCBuY29sdW1ucykKIAkJCQlyZWxvaWQgPSBlbnRyeS0+
a2V5Lm9iam9pZDsKIAogCQkJCWlmICh0YWJlbnRyeSAhPSBOVUxMICYmIHRhYmVudHJ5LT52YWN1
dW1fZXh0LnR5cGUgPT0gdHlwZSkKLQkJCQkJdHVwbGVzdG9yZV9wdXRfZm9yX3JlbGF0aW9uKHJl
bG9pZCwgdHVwc3RvcmUsIHR1cGRlc2MsIHRhYmVudHJ5LCBuY29sdW1ucyk7CisJCQkJCXR1cGxl
c3RvcmVfcHV0X2Zvcl9yZWxhdGlvbihyZWxvaWQsIHJzaW5mbywgdGFiZW50cnksIG5jb2x1bW5z
KTsKIAkJCX0KIAkJfQogCX0KQEAgLTIyODIsNyArMjI2Myw3IEBAIHBnX3N0YXRzX3ZhY3V1bShG
dW5jdGlvbkNhbGxJbmZvIGZjaW5mbywgRXh0VmFjUmVwb3J0VHlwZSB0eXBlLCBpbnQgbmNvbHVt
bnMpCiAJCQkJLyogVGFibGUgZG9lc24ndCBleGlzdCBvciBpc24ndCBhIGhlYXAgcmVsYXRpb24g
Ki8KIAkJCQlQR19SRVRVUk5fTlVMTCgpOwogCi0JCQl0dXBsZXN0b3JlX3B1dF9mb3JfZGF0YWJh
c2UoZGJpZCwgdHVwc3RvcmUsIHR1cGRlc2MsIGRiZW50cnksIG5jb2x1bW5zKTsKKwkJCXR1cGxl
c3RvcmVfcHV0X2Zvcl9kYXRhYmFzZShkYmlkLCByc2luZm8sIGRiZW50cnksIG5jb2x1bW5zKTsK
IAkJCXBnc3RhdF91bmxvY2tfZW50cnkoZW50cnlfcmVmKTsKIAkJfQogCQllbHNlCi0tIAoyLjM0
LjEKCg==
--00000000000068faff061fcb0829--