Re: [Patch] Windows relation extension failure at 2GB and 4GB

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Bryan Green <dbryan(dot)green(at)gmail(dot)com>
Cc: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [Patch] Windows relation extension failure at 2GB and 4GB
Date: 2025-11-14 06:44:50
Message-ID: aRbP4uSwd0Edpp5S@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Nov 13, 2025 at 10:58:54AM -0600, Bryan Green wrote:
> On 11/12/2025 10:05 PM, Michael Paquier wrote:
>> > Moving on to the I/O routine changes. There was a little bit of
>> noise in the diffs, like one more comment removed that should still be
>> around. Indentation has needed some adjustment as well, there was one
>> funny diff with a cast to pgoff_t. And done this part as a first
>> step, because that's already a nice cut.
>
> Apologies for the state of this and your loss of time. I was testing a
> new workflow and attached the wrong revision. No excuse, just what
> happened. I will be more careful and do a closer review of diffs going
> forward.

No worries. Thanks for all your efforts here.

> I had started down the path of using sql and doing regression testing
> and decide instead that a tap test would be better for my specific case
> of testing on Windows.

How much do we really care about the case of FSCTL_SET_SPARSE? We
don't use it in the tree, and I doubt we will, but perhaps you have
some plans to use it for something I am unaware of, that would justify
its existence?

> The argument for a TAP test in this case would be File::Temp handles
> cleanup automatically for us (even on test failure). Also, no need for
> alternate output files.
>
> I agree we should go to a cross-platform test. I'm 51/49 in favor of
> using TAP tests still, but if you, or others, feel strongly otherwise, I
> can restructure it to work that way.

There are a couple of options here:
- Use NO_INSTALLCHECK so as the test would never be run on an existing
deployment, only check. We could use that on top of a PG_TEST_EXTRA
to check with a large offset if the writes cannot be cheap..
- For alternate output, the module could have a SQL function that
returns the size of off_t or equivalent, mixed with an \if to avoid
the test for a sizeof 4 bytes.

If others argue in favor of a TAP test as well, that's OK by me.
However, there is nothing in the current patch that justifies that:
the proposal only does direct SQL function calls and does not need a
specific configuration or any cluster manipulations (aka restarts,
etc).
--
Michael

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2025-11-14 06:51:22 Re: Document NULL
Previous Message Peter Smith 2025-11-14 06:44:43 Re: Skipping schema changes in publication