Re: Loaded footgun open_datasync on Windows

From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Loaded footgun open_datasync on Windows
Date: 2018-06-08 17:30:44
Message-ID: CABUevEzK+y93kaGgpfOp=233LnSQgym9U_S2D676EGd4aDuRLA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jun 8, 2018 at 4:55 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:

> On Fri, Jun 8, 2018 at 7:48 AM, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
> wrote:
>
>> Amit Kapila wrote:
>> > On Wed, Jun 6, 2018 at 3:06 PM, Kuntal Ghosh <
>> kuntalghosh(dot)2007(at)gmail(dot)com> wrote:
>> > > It seems the "#ifndef FRONTEND" restriction was added around
>> > > pgwin32_open() for building libpq with Visual C++ or Borland C++. The
>> > > restriction was added in commit 422d4819 to build libpq with VC++[1].
>> > > Later, in commit fd7c3f67e0bc4, the support for Borland C++ was also
>> > > added.
>> > >
>> > > So, I'm not sure whether removing that restriction will work for all
>> > > front-end modules.
>> > >
>> >
>> > Thanks for doing investigation. I agree with you that it appears from
>> the old
>> > discussion that we have added this restriction to build libpq/psql
>> (basically frontend)
>> > modules. However, I tried to build those modules on windows by
>> removing that
>> > restriction and it doesn't give me any problem (except one extra
>> argument error,
>> > which can be easily resolved). It might be that it gives problem with
>> certain
>> > version of MSVC. I think if we want to pursue this direction, one
>> needs to verify
>> > on various supportted versions (of Windows) to see if things are okay.
>>
>> That's what the buildfarm is for, right?
>>
>> > Speaking about the issue at-hand, one way to make pg_test_fsync work in
>> to just
>> > include c.h and remove inclusion of postgres_fe.h, but not sure if that
>> is the best way.
>> > I am not sure if we have any other options to proceed in this case.
>> >
>> > Thoughts?
>>
>> I thought of a better way.
>> pg_test_fsync is properly listed as a server application in the
>> documentation,
>> so I think it should be built as such, as per the attached patch.
>>
>
> -#include "postgres_fe.h"
> +#include "postgres.h"
>
> I don't think that server application can use backend environment unless
> it is adapted to do so. I think the application should have the capability
> to deal with backend stuff like ereport, elog etc, if we don't want to use
> FRONTEND environment. The other server applications like pg_ctl.c,
> initdb.c, etc. also uses FRONTEND environment.
>
>
Not having researched this particular case but yes, there are a number of
things expected in a non-FRONTEND environment. Things that may also go
unnoticed until too late (e.g. singal handling etc that needs explicit
initialization). Standalong binaries should pretty much always be
frontend.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2018-06-08 17:41:40 Re: Needless additional partition check in INSERT?
Previous Message Tom Lane 2018-06-08 16:57:39 Re: Internal error XX000 with enable_partition_pruning=on, pg 11 beta1 on Debian