Magnus Hagander <magnus@hagander.net> writes:
> A whole lot simpler patch :-)

Seems like you no longer need the !defined(_DIRMOD_C) bit here?
Also please include a comment about why <sys/stat.h> has to be
forcibly included.

A more general question: can't we get rid of most of the #ifdef WIN32
cruft in include/port.h, and put it in include/port/win32.h instead?
Seems cleaner that way, at least for things where there's just an
#ifdef WIN32 hunk and not two cases for Win and not-Win.

regards, tom lane

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Andrew Dunstan wrote:
>> How about #defining safe_stat to be pg_win32_safe_stat on Windows and
>> simply stat elsewhere? Then use safe_stat at the places you consider
>> critical.

> I would couple this with a pgwin32_unsafe_stat on Windows, which changes
> the size value to 0, so that if anyone gets it wrong it's immediately
> obvious.

It's only worth having two versions if someone can show that there's
actually going to be a performance problem from the extra syscall.
I don't believe we use stat() in any place where it's really gonna
matter much ...

regards, tom lane

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
> > A whole lot simpler patch :-)
>
> Seems like you no longer need the !defined(_DIRMOD_C) bit here?

Correct. That wasn't the actual error, that was me misdiagnosing the
situation.

> Also please include a comment about why <sys/stat.h> has to be
> forcibly included.

Will do.


> A more general question: can't we get rid of most of the #ifdef WIN32
> cruft in include/port.h, and put it in include/port/win32.h instead?
> Seems cleaner that way, at least for things where there's just an
> #ifdef WIN32 hunk and not two cases for Win and not-Win.

Yeah, that one has been on my TODO for a while.

We may not be able to move everything because one file is included
early in the pass of c.h and one much later, but the majority of the
stuff we have there shouldn't care about which order it goes in at.

//Magnus

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Magnus Hagander wrote:
> Magnus Hagander wrote:
> > Attached is a patch that attempts to fix the issues with stat() not
> > properly updating st_size on win32, as reported in this thread:
> > http://archives.postgresql.org/pgsql...3/msg01181.php
> >
> > It has to have a chance to affect things beyond just the
> > pg_relation_size() function, so this patch fixes all cases where we
> > do stat() and use the st_size member. It doesn't change all
> > occurances of stat() since I didn't want to incur the double
> > filesystem lookups unnecessary cases.
> >
> > Any objections?
>
> Updated version. Seems the problem was that the includes came in the
> wrong order - figured that out just after I went to bed Here's an
> updated patch.
>
> A whole lot simpler patch :-)

Appled with comment adjustment, and backpatched to 8.2 and 8.3.

//Magnus

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches