|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|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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.
> 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
Juan José Santamaría Flecha
|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|
|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|