Re: Do we want to avoid checksumming extra files in the datadir? [was: BUG #19647]

From: Shubhra Jain <shubhra(dot)jain(at)ksolves(dot)com>
To: Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Daniel Gustafsson <daniel(at)yesql(dot)se>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Edwin Polkerman <edwin(dot)polkerman(at)splendiddata(dot)com>
Subject: Re: Do we want to avoid checksumming extra files in the datadir? [was: BUG #19647]
Date: 2026-10-07 11:04:00
Message-ID: CAOh5eDXQgUqrUG7j+C04Wn5w4rNEOMaqU23ms_G7jay9s=mUtg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Hi Jacob, all,

I applied v2-0001..0003 on top of master (e40b509c08) and tested them.

- pg_checksums (77 tests) and pg_basebackup (352 tests) TAP tests pass.
- With stray files in global/ ("foreign_file" with 4 bytes and
"foreign.conf"), unpatched pg_checksums fails ("could not read block
0 ... read 4 of 8192"; the dotted name gave "invalid segment number
0"), while v2 exits 0.
- On a cluster with a table, an index and an unlogged table, patched
and unpatched report identical counts (956 files, 9362 blocks).
- "--filenode 0" is rejected with "must be in range 1..2147483647".

Two things I noticed while reviewing:

1. doc/src/sgml/ref/pg_checksums.sgml still says that when verifying
checksums "every file in the cluster is scanned". With 0003 that is
no longer accurate; it could say "every relation file" and mention
that other files are ignored.

2. Per Rui's review, the 0003 commit message could mention that
--filenode is now compared numerically, and a leading-zero
--filenode case is not yet in check_relation_corruption().

I'm happy to write the docs and test changes if you'd like.

Thanks,
Shubhra

On Wed, Oct 7, 2026 at 4:32 PM Jacob Champion <
jacob(dot)champion(at)enterprisedb(dot)com> wrote:

> On Thu, Sep 3, 2026 at 11:30 AM Jacob Champion
> <jacob(dot)champion(at)enterprisedb(dot)com> wrote:
> > Sounds good, thanks both! Attached is the simplest thing that could
> > fix the reported problem (and nothing else), but I'd rather look into
> > moving parse_filename_for_nontemp_relation() to common/relfile.c so it
> > can be used directly. I probably won't have time for that today.
>
> Here's a v2 to do that, which I like much better.
>
> The --filenode argument now filters via integer equality rather than a
> string comparison. I think there are two main side effects (let me
> know if either is unacceptable):
> 1) we now prohibit `pg_checksums --filenode 0`, which IIUC isn't ever
> helpful in practice, and
> 2) leading zeroes in the --filenode arg are now ignored, rather than
> causing pg_checksums to match nothing. I.e. `--filenode 001260` will
> now match relfile 1260.
>
> Are there are any corner cases I've missed where you want pg_checksums
> to check a temporary relation's relfiles? Even if a clean shutdown and
> startup somehow left them around, they still wouldn't be used, right?
>
> Thanks,
> --Jacob
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Shlok Kyal 2026-10-07 11:19:01 Re: Parallel Apply
Previous Message Ayoub Kazar 2026-10-07 11:02:23 [PROPOSAL] Expand OR clauses in joins to UNION ALL paths

Browse pgsql-bugs by date

  From Date Subject
Previous Message Zhijie Hou 2026-10-07 10:08:36 Re: Streaming decoding fails with "unexpected table_index_fetch_tuple call during logical decoding" when a relation has a TOASTed conbin (follow-up to BUG #18641)