On Fri, Nov 7, 2025 at 11:29 AM Michael Paquier <michael@paquier.xyz> wrote:
> On Thu, Nov 06, 2025 at 11:17:52AM -0600, Bryan Green wrote:
> > Latest patch attached that includes these code paths.
>
> That feels OK for me. Thomas, do you have a different view on the
> matter for HEAD? Like long, I would just switch to something that we
> have in the tree that's fixed.
WFM.
- /* Note that this changes the file position, despite not using it. */
Why drop these comments? They still apply.
-
Accidental whitespace change?
struct radvisory
{
- off_t ra_offset; /* offset into the file */
+ pgoff_t ra_offset; /* offset into the file */
IIRC this is a struct definition from an Apple man page, maybe leave unchanged?
Looking at the v3 that arrived while I was typing this:
+ errmsg("sparse files are only supported on Windows")));
Nit: maybe sparse file test only supported on Windows? Also, nice test!
> And +1 for the idea to restrict the segment size to never be more than
> 2GB based on a ./configure and meson check on the back branches. In
> PG15 and older branches, we already enforced a check by the way. See
> src/tools/msvc/Solution.pm which was the only way to compile the code
> with visual studio so one would have never seen the limitations except
> if they had the idea to edit the perl scripts (FWIW, I've done exactly
> that in the past for a past project at $company, never touched the
> segsize):
> # only allow segsize 1 for now, as we can't do large files yet in windows
> die "Bad segsize $options->{segsize}"
> unless $options->{segsize} == 1;
>
> So this is a meson issue that goes down to v16, when using a VS
> compiler. Was there a different compiler where off_t is also 4 bytes?
Ohh, this all makes more sense now. I wasn't wrong to think there was
already a check, it just didn't get ported to meson.
I wouldn't personally pitch the commit message as "Fix ...", which
sounds like a bug fix. There *is* a bug, but it's in the meson work.
Something more like "Allow large relation files on Windows" seems more
appropriate for this one, but YMMV.
> MinGW is mentioned as clear by Thomas.
Only MinGW + meson. MinGW + configure has 32-bit off_t as far as I
can tell because we do:
if test "$PORTNAME" != "win32"; then
AC_SYS_LARGEFILE
...
I don't personally know of any current Unix without LFS, they just
vary on whether it's always on or you have to ask for it, as autoconf
and meson know. But I suppose the check for oversized segments should
use sizeof(off_t), not the OS's identity.
There are of course a few filesystems even today that don't let you
make a file as big as our maximum size, but that's another topic.
