Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1siFrK-001EDY-1S for pgsql-hackers@arkaria.postgresql.org; Sun, 25 Aug 2024 16:12:54 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1siFrH-00GhRd-PL for pgsql-hackers@arkaria.postgresql.org; Sun, 25 Aug 2024 16:12:52 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1siFrH-00GhRU-6w for pgsql-hackers@lists.postgresql.org; Sun, 25 Aug 2024 16:12:52 +0000 Received: from mail.postgrespro.ru ([93.174.131.139]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1siFr9-001NtW-NS for pgsql-hackers@postgresql.org; Sun, 25 Aug 2024 16:12:50 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=postgrespro.ru; s=mx2023; t=1724602361; bh=3zbY4ceZ2hx6pQwwpzSI0Qwg8Z5Hpa8oWPxyt1vqCc4=; h=Message-ID:Date:User-Agent:Subject:To:Cc:References:From: In-Reply-To:From; b=TN0F4HwWTQsY52XEM0t+s7yWfbAr/+PKgNDchQhLxJFu6F5m7jLv93jlyRuawF+Ms HSU1qsVLCMuHW2yAtVxLKQ7jO/LmPoQyJvxMBBdQcfrxYpKrIjO4yR+ZlNTpytZS2o CG7CADpU0SnfAKnHcLx2fXCwQbF+DgV3ru+trjrrMwF3wszidYyRSaRg6N2rQqxWs4 b+henbIx5gHXN0mAUf8UnxotMUclW2fFBg/cdq3Mr27xJRRCfwByYP3TxfxkVa7/Fe zBLX8tbAal6W4rcxT9Evhdfbk8N6lJH/cT/0DbbX4AUILJC6tgxe8CRuiqgmmaEIUR jqWTeFl+fHd5g== Received: from [192.168.15.77] (unknown [45.137.112.9]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: a.rybakina@postgrespro.ru) by mail.postgrespro.ru (Postfix/587) with ESMTPSA id C2A646015F; Sun, 25 Aug 2024 19:12:40 +0300 (MSK) Content-Type: multipart/alternative; boundary="------------hmkyUYKLZcJheRK7QppSuT50" Message-ID: Date: Sun, 25 Aug 2024 19:12:40 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: Vacuum statistics To: Kirill Reshke Cc: jian he , Ilia Evdokimov , Andrei Zubkov , Alena Rybakina , pgsql-hackers , a.lepikhov@postgrespro.ru References: <53c47c2d-72a5-44f2-900c-9973b2af1808@tantorlabs.com> <4a902cea-54fb-41b5-b208-b84731a5f577@postgrespro.ru> <78394e29-a900-4af4-b5ce-d6eb2d263fad@postgrespro.ru> Content-Language: en-US From: Alena Rybakina In-Reply-To: X-KSMG-AntiPhishing: NotDetected, bases: 2024/08/25 15:22:00 X-KSMG-AntiSpam-Interceptor-Info: not scanned X-KSMG-AntiSpam-Status: not scanned, disabled by settings X-KSMG-AntiVirus: Kaspersky Secure Mail Gateway, version 2.1.0.7854, bases: 2024/08/25 14:21:00 #26453186 X-KSMG-AntiVirus-Status: NotDetected, skipped X-KSMG-LinksScanning: not scanned, disabled by settings X-KSMG-Message-Action: skipped X-KSMG-Rule-ID: 1 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk This is a multi-part message in MIME format. --------------hmkyUYKLZcJheRK7QppSuT50 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 22.08.2024 07:29, Kirill Reshke wrote: > On Thu, 22 Aug 2024 at 07:48, jian he wrote: >> On Wed, Aug 21, 2024 at 6:37 AM Alena Rybakina >> wrote: >>> We check it there: "tabentry->vacuum_ext.type != type". Or were you talking about something else? >>> >>> On 19.08.2024 12:32, jian he wrote: >>> >>> in pg_stats_vacuum >>> if (type == PGSTAT_EXTVAC_INDEX || type == PGSTAT_EXTVAC_HEAP) >>> { >>> Oid relid = PG_GETARG_OID(1); >>> >>> /* Load table statistics for specified database. */ >>> if (OidIsValid(relid)) >>> { >>> tabentry = fetch_dbstat_tabentry(dbid, relid); >>> if (tabentry == NULL || tabentry->vacuum_ext.type != type) >>> /* Table don't exists or isn't an heap relation. */ >>> PG_RETURN_NULL(); >>> >>> tuplestore_put_for_relation(relid, rsinfo, tabentry); >>> } >>> else >>> { >>> } >>> >>> >>> So for functions pg_stat_vacuum_indexes and pg_stat_vacuum_tables, >>> it seems you didn't check "relid" 's relkind, >>> you may need to use get_rel_relkind. >>> >>> -- >> hi. >> I mentioned some points at [1], >> Please check the attached patchset to address these issues. >> >> there are four occurrences of "CurrentDatabaseId", i am still confused >> with usage of CurrentDatabaseId. >> >> also please don't top-post, otherwise the archive, like [2] is not >> easier to read for future readers. >> generally you quote first, then reply. >> >> [1]https://postgr.es/m/CACJufxHb_YGCp=pVH6DZcpk9yML+SueffPeaRbX2LzXZVahd_w@mail.gmail.com >> [2]https://postgr.es/m/78394e29-a900-4af4-b5ce-d6eb2d263fad@postgrespro.ru > Hi, your points are valid. > Regarding 0003, I also wanted to object database naming in a > regression test during my review but for some reason didn't.Now, as > soon as we already need to change it, I suggest we also change > regression_statistic_vacuum_db1 to something less generic. Maybe > regression_statistic_vacuum_db_unaffected. > Hi! I fixed it in the new version of the patch [0]. Feel free to review it! To be honest, I still doubt that the current database names (regression_statistic_vacuum_db and regression_statistic_vacuum_db1) can be easily generated, but if you insist on renaming, I will do it. [0] https://www.postgresql.org/message-id/c4e4e305-7119-4183-b49a-d7092f4efba3%40postgrespro.ru -- Regards, Alena Rybakina Postgres Professional:http://www.postgrespro.com The Russian Postgres Company --------------hmkyUYKLZcJheRK7QppSuT50 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit
On 22.08.2024 07:29, Kirill Reshke wrote:
On Thu, 22 Aug 2024 at 07:48, jian he <jian.universality@gmail.com> wrote:

On Wed, Aug 21, 2024 at 6:37 AM Alena Rybakina
<a.rybakina@postgrespro.ru> wrote:

We check it there: "tabentry->vacuum_ext.type != type". Or were you talking about something else?

On 19.08.2024 12:32, jian he wrote:

in pg_stats_vacuum
    if (type == PGSTAT_EXTVAC_INDEX || type == PGSTAT_EXTVAC_HEAP)
    {
        Oid                    relid = PG_GETARG_OID(1);

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

            tuplestore_put_for_relation(relid, rsinfo, tabentry);
        }
        else
        {
       }


So for functions pg_stat_vacuum_indexes and pg_stat_vacuum_tables,
it seems you didn't check "relid" 's relkind,
you may need to use get_rel_relkind.

--

hi.
I mentioned some points at [1],
Please check the attached patchset to address these issues.

there are four occurrences of "CurrentDatabaseId", i am still confused
with usage of CurrentDatabaseId.

also please don't  top-post, otherwise the archive, like [2] is not
easier to read for future readers.
generally you quote first, then reply.

[1] https://postgr.es/m/CACJufxHb_YGCp=pVH6DZcpk9yML+SueffPeaRbX2LzXZVahd_w@mail.gmail.com
[2] https://postgr.es/m/78394e29-a900-4af4-b5ce-d6eb2d263fad@postgrespro.ru

Hi, your points are valid.
Regarding 0003, I also wanted to object database naming in a
regression test during my review but for some reason didn't.Now, as
soon as we already need to change it, I suggest we also change
regression_statistic_vacuum_db1 to something less generic. Maybe
regression_statistic_vacuum_db_unaffected.

Hi! I fixed it in the new version of the patch [0]. Feel free to review it!

To be honest, I still doubt that the current database names (regression_statistic_vacuum_db and regression_statistic_vacuum_db1) can be easily generated, but if you insist on renaming, I will do it.

[0] https://www.postgresql.org/message-id/c4e4e305-7119-4183-b49a-d7092f4efba3%40postgrespro.ru

-- 
Regards,
Alena Rybakina
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
--------------hmkyUYKLZcJheRK7QppSuT50--