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

On Thu, Nov 27, 2025 at 01:52:33AM -0600, Bryan Green wrote:
> The test now uses OpenTemporaryFile() via the VFD layer, which handles
> cleanup automatically, so there's no need for TAP's File::Temp. A
> test_large_files_offset_size() function returns sizeof(pgoff_t), and
> the SQL uses \if to skip on platforms where that's less than 8 bytes.
> NO_INSTALLCHECK is set.
> One issue came up during testing: at 2GB+1, the OVERLAPPED.OffsetHigh
> field is naturally zero, so commenting out the OffsetHigh fix didn't
> cause the test to fail. I've changed the test offset to 4GB+1 where
> OffsetHigh must be non-zero. The test now catches both bugs. FileSize()
> provides independent verification that writes actually reached the
> correct offset.
>
> I have changed the name of the patch to reflect that it is not just
> adding tests, but includes the change for the problem.
>
> Updated patch attached.

Pretty cool result, and the test fails in the Windows CI with
84fb27511dbe reverted:
- test_large_files_test_4gb_boundary
-------------------------------------
- 4GB boundary test passed
-(1 row)
-
+ERROR: file size is 9 bytes, expected at least 4294967305 bytes -
offset truncated

-# If we don't have largefile support, can't handle segment size >= 2GB.
-if cc.sizeof('off_t', args: test_c_args) < 8
- segsize_bytes = segsize * blocksize
- if segsize_bytes >= (2 * 1024 * 1024 * 1024)
- error('Large file support is not enabled. Segment size cannot be larger than 1GB.')
- endif
-endif

I doubt that we can drop this check yet. There are still a lot of
places in the tree that need to be switched from off_t to pgoff_t,
like the buffer APIs, etc.

+SELECT test_large_files_offset_size() >= 8 AS has_large_file_support \gset
+-- Only run test on platforms with 64-bit offsets
+\if :has_large_file_support

That would be sufficient to make the test conditional. However we
already know that pgoff_t is forced at 8 bytes all the time in the
tree, so why do we need that. If that was based on off_t, with tests
around it, that would be adapted, of course.

+PG_FUNCTION_INFO_V1(test_large_files_test_4gb_boundary);
+Datum
+test_large_files_test_4gb_boundary(PG_FUNCTION_ARGS)

As of this patch, this includes one function that opens a temporary
file, writes some data into it twice, checks its size, reads a few
bytes, closes the file. When designing such test modules, it is
important to make them modular, IMO, so as they can be extended at
will for more cases in the future. How about introducing a set of
thin SQL wrappers around all these File*() functions, taking in input
what they need. For the data written, this would be some bytea in
input combined with an offset. For the data read, a chunk of data to
return, with an offset where the data was read from. Then the
comparisons could be done on the two byteas, for example. FileClose()
is triggered at transaction commit through CleanupTempFiles(), so we
could return the File as an int4 when passed around to the SQL
functions (cannot rely on pg_read_file() as the path is not fixed),
passing it in input of the read, write and close function (close is
optional as we could rely on an explicit commit).

The new module is missing a .gitignore, leading to files present in
the tree when using configure/make after a test run. You could just
copy one from one of the other test modules.
--
Michael