Re: pg_dump/pg_restore: Fix stdin/stdout handling of custom format on Win32

From: Daniel Watzinger <daniel(dot)watzinger(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Juan José Santamaría Flecha <juanjo(dot)santamaria(at)gmail(dot)com>, Daniel Gustafsson <daniel(at)yesql(dot)se>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pg_dump/pg_restore: Fix stdin/stdout handling of custom format on Win32
Date: 2023-03-14 11:30:18
Message-ID: CAD9KL3OvAmfY-gAh2Fgn-5FZCq7ugHqAoTfe=AXuNFSRC5f16A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I'm sorry I couldn't contribute to the discussion in time. The fix of the
fstat() Win32 port looks good to me. I agree that there's a need for
multiple fseek() ports to address the shortcomings of the MSVC
functionality.

The documentation event states that "on devices incapable of seeking, the
return value is undefined". A simple wrapper using GetFileType() or the new
fstat(), to filter non-seekable devices before delegation, will probably
suffice.

https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fseek-fseeki64?view=msvc-170

Regarding test automation and regression testing, there's a programmatic
way to simulate how the "pipe operator" of cmd.exe and other shells works
using CreateProcess and manual "piping" by means of various WinAPI
functionality. This is actually how the bug was discovered in the first
case. However, existing tests are probably platform-agnostic.

--
Daniel

On Tue, Mar 14, 2023 at 12:41 AM Michael Paquier <michael(at)paquier(dot)xyz>
wrote:

> On Mon, Mar 13, 2023 at 05:49:41PM +0100, Juan José Santamaría Flecha
> wrote:
> > WFM, making fseek() behaviour more resilient seems like a good
> improvement
> > overall.
>
> I have not looked in details, but my guess would be to add a
> win32seek.c similar to win32stat.c with a port of fseek() that's more
> resilient to the definitions in POSIX.
>
> > Should we open a new thread to make that part more visible?
>
> Yes, perhaps it makes sense to do so to attract the correct audience,
> There may be a few things we are missing.
>
> When it comes to pg_dump, both fixes are required, still it seems to
> me that adjusting the fstat() port and the fseek() ports are two
> different bugs, as they influence different parts of the code base
> when taken individually (aka this fseek() port for WIN32 would need
> fstat() to properly detect a pipe, as far as I understand).
>
> Meanwhile, I'll go apply and backpatch 0001 to fix the first bug at
> hand with the fstat() port, if there are no objections.
> --
> Michael
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2023-03-14 11:32:39 Re: Add macros for ReorderBufferTXN toptxn
Previous Message John Naylor 2023-03-14 11:27:37 Re: [PoC] Improve dead tuple storage for lazy vacuum