On Thu, Dec 18, 2025 at 1:05 AM Amul Sul <[email protected]> wrote:
> Test looks good to me, but I have three suggestions as follow:
>
> 1. To minimize repetition in insert: use fillfactor 10, which is the
> minimal we can set for a table, so that we can minimize tuples per
> page. Use a longer string and lower count in repeat(), which I believe
> helps the test become a bit faster.

I haven't checked how big a relation the test case creates, but it's
worth keeping in mind that the CI tests run on one platform with the
segment size set to six blocks. I think we should design the test case
with that in mind i.e. don't worry about catching the bug when the
segment size is 1GB, but make sure the test fails in CI without the
bug fix. Let's not rely on fillfactor -- the cost here is the disk
space and the time to write the blocks, not how many tuples they
actually contain.

> 2. I think we could add this test to the existing pg_combinebackup's
> test file instead of creating a new file with a single-test. See the
> attached version; it’s a bit smaller than your original patch, but
> since I haven't copied all of your comments yet, I’ve marked it as
> WIP.

-1. This kind of thing tends to make the tests harder to understand.

-- 
Robert Haas
EDB: http://www.enterprisedb.com


Reply via email to