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

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