Date: Thu, 09 Jul 2026 16:15:42 +0900 (JST)
Message-Id: <20260709.161542.46102955322725819.horikyota.ntt@gmail.com>
To: masao.fujii@gmail.com
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Fix data checksum processing for temp relations and dropped
 databases
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: 
 <CAHGQGwGDHAQw=bmpRzk+EmKzVtxZiD5YDurMUffBMwr6WXugQA@mail.gmail.com>
References: 
 <CAHGQGwGDHAQw=bmpRzk+EmKzVtxZiD5YDurMUffBMwr6WXugQA@mail.gmail.com>
User-Agent: Mew version 6.8 on Emacs 29.4
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20260709.161542.46102955322725819.horikyota.ntt%40gmail.com>
Precedence: bulk

Hello,

At Thu, 9 Jul 2026 11:24:31 +0900, Fujii Masao <masao.fujii@gmail.com> wrote in 
> I think BuildRelationList() should exclude temporary relations without
> RELKIND_HAS_STORAGE(), matching the existing behavior for
> non-temporary relations. Attached patch implements this.
...
> fails without processing the shared catalogs, yet they are still marked
> as processed. As a result, later workers skip them, and checksum
> enabling can complete successfully even though the shared catalogs
> were never processed.
> 
> To fix this, we should mark the shared catalogs as processed only after
> a worker completes successfully. Attached patch also implements this.
> 
> Thoughts?

The changes look functionally correct to me.

A couple of minor comments:

1. In BuildRelationList(), if relations without storage should be
 excluded regardless of whether they are temporary or non-temporary, I
 would personally prefer moving the RELKIND_HAS_STORAGE() check before
 the persistence check. That would make the intent a bit clearer.

2. In ProcessAllDatabases(), I find it slightly easier to read if the
 dropped-database case is handled explicitly first, perhaps by
 continuing early, so that DATACHECKSUMSWORKER_SUCCESSFUL remains the
 default result for the normal path. I'm not completely sure whether
 that check should go just before or just after
 pgstat_progress_update_param(), though.

Both are mostly matters of style, so please feel free to ignore them
if you prefer the current structure.

Regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center