Hello,

At Thu, 9 Jul 2026 11:24:31 +0900, Fujii Masao <[email protected]> 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


Reply via email to