Re: BUG #15858: could not stat file - over 4GB

From: Juan José Santamaría Flecha <juanjo(dot)santamaria(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, williamedwinallen(at)live(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org, Magnus Hagander <magnus(at)hagander(dot)net>
Subject: Re: BUG #15858: could not stat file - over 4GB
Date: 2019-06-28 21:34:38
Message-ID: CAC+AXB2VAQ+ZTxXKStyijEac6rJOa0HvSEOikfF4+Pm5Q_3dvg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On Wed, Jun 26, 2019 at 4:23 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> The former patch was rather impressive. Or scary. Or both. At which
> extent have you tested it? I think that we really need to make sure
> of a couple of things which satisfy our needs:

I wanted to make a quick test on the previous patch. So let me state
what have I tested and what I have not: it builds and pass tests in
Windows and Cygwin, but I have not setup a MinGW environment.

> 1) Are we able to fix the issues with stat() calls on files larger
> than 2GB and report a correct size?

I have successfuly tested a COPY with large files.

> 2) Are we able to detect properly that files pending for deletion are
> discarded with ENOENT?

Cannot reproduce reliably, but is using the same logic as pgwin32_safestat().

> 3) Are frontends able to use the new layer?

After removing UNSAFE_STAT_OK, is this still an issue?

> It seems to me that you don't need the configure changes.

The changes in configuration are meant for gcc compilations in Windows
(Cygwin and Mingw).

> Instead of stat_pg_fixed which is confusing because it only involves
> Windows, I would rename the new file to stat.c or win32_stat.c. The
> location in src/port/ is adapted. I would also move out of
> win32_port.h the various inline declarations and keep only raw
> declarations. That could be much cleaner.

Ok.

> The code desperately needs more comments to help understand its
> logic. Don't we have in the tree an equivalent of cvt_ft2ut? What
> does cvt_attr2uxmode do? It would be nice to avoid conversion
> wrappers as much as possible, and find out system-related equivalents
> if any, and actually if necessary.

I have only found something similar in ./src/port/gettimeofday.c, but
not sure if this patch should touch that code.

> +static unsigned short
> +cvt_attr2uxmode(int attr, const _TCHAR * name)
> This looks rather bug-prone...

I wanted to keep as much of the original code as possible, but if this
is found as a viable solution, what shape should it have?

> I think that this stuff has not been tested and would break at
> compilation. If src/tools/msvc/Mkvcbuild.pm is not changed, then the
> new file won't get included in the compiled set.

The previous patch was broken, taken from the wrong local branch
(sorry about that). The attached is still a WIP but it has to do the
things above-mentioned.

Regards,

Juan José Santamaría Flecha

Attachment Content-Type Size
0001-WIP-support-for-large-files-on-Win32-v3.patch application/octet-stream 9.1 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Alvaro Herrera 2019-06-28 21:56:03 Re: BUG #15677: Crash while deleting from partitioned table
Previous Message Alvaro Herrera 2019-06-28 21:26:10 Re: BUG #15383: Join Filter cost estimation problem in 10.5

Browse pgsql-hackers by date

  From Date Subject
Next Message David Steele 2019-06-28 21:47:20 Re: Commitfest 2019-07, the first of five* for PostgreSQL 13
Previous Message Alvaro Herrera 2019-06-28 21:22:24 Re: BUG #15293: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events