Subject: Fix data checksum processing for temp relations and dropped databases

From: Fujii Masao <[email-masked]>
Date: Thu, 9 Jul 2026 11:24:31 +0900

Hi,

While reading the data checksum code, I found that BuildRelationList()
filters out non-temporary relations without storage, but does not
apply the same filtering to temporary relations. As a result, temporary
relations without storage, such as temporary views, are included in
the list of relations to wait for even though they are irrelevant to
checksum processing. For example, enabling data checksums can
unnecessarily wait for a long-lived session that owns only a temporary view.

I think BuildRelationList() should exclude temporary relations without
RELKIND_HAS_STORAGE(), matching the existing behavior for
non-temporary relations. Attached patch implements this.

Also there seems a small race condition. When enabling data checksums
online, the launcher assigns the first worker to process the shared catalogs
and prevents later workers from doing so. However, if that worker's
database is dropped after it has been selected for processing but
before checksum processing begins (a very small window), the worker
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?

Regards,

-- 
Fujii Masao

---

From: Kyotaro Horiguchi <[email-masked]>
Date: Thu, 09 Jul 2026 16:15:42 +0900 (JST)

Hello,

At Thu, 9 Jul 2026 11:24:31 +0900, Fujii Masao <[email-masked]> wrote in 
> non-temporary relations. Attached patch implements this.
...
> a worker completes successfully. Attached patch also implements this.

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

---

From: Daniel Gustafsson <[email-masked]>
Date: Thu, 9 Jul 2026 09:56:44 +0200

> On 9 Jul 2026, at 09:15, Kyotaro Horiguchi <[email-masked]> wrote:

> The changes look functionally correct to me.

Agreed.

> the persistence check. That would make the intent a bit clearer.

Makes sense, though I am fine with either approach.

> pgstat_progress_update_param(), though.

I like this suggestion, though I think it should go after the call to
pgstat_progress_update_param since the db in question reached ProcessDatabase.
I would add it as an else-if case to the conditional testing we have for
_FAILED and _ABORTED.

--
Daniel Gustafsson