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
