Re: Add pg_file_sync() to adminpack - Mailing list pgsql-hackers

From Artur Zakirov
Subject Re: Add pg_file_sync() to adminpack
Date
Msg-id CAKNkYnyC2F+OZgYs0L8QmBBbGwdTZHtyx-b18zkf-RnhybsVXg@mail.gmail.com
Whole thread Raw
In response to Re: Add pg_file_sync() to adminpack  (Fujii Masao <masao.fujii@gmail.com>)
List pgsql-hackers
Hello,

On Sat, Jan 11, 2020 at 2:12 AM Fujii Masao <masao.fujii@gmail.com> wrote:
> > + <para>
> > +  <function>pg_file_sync</function> fsyncs the specified file or directory
> > +  named by <parameter>filename</parameter>.  Returns true on success,
> > +  an error is thrown otherwise (e.g., the specified file is not present).
> > + </para>
> > What's the point of having a function that returns a boolean if it
> > just returns true all the time?  Wouldn't it be better to have a set
> > of semantics closer to the unlink() part, where the call of stat()
> > fails with an ERROR for (errno != ENOENT) and the fsync call returns
> > false with a WARNING?
>
> I'm not sure if returning false with WARNING only in some error cases
> is really good idea or not. At least for me, it's more intuitive to
> return true on success and emit an ERROR otherwise. I'd like to hear
> more opinions about this. Also if returning true on success is rather
> confusing, we can change its return type to void.

I think it would be more consistent to pg_file_unlink().

Other functions throw an ERROR and return a number or set of records
except pg_file_rename(), which in some cases throws a WARNING and
returns a boolean result.

-- 
Arthur



pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: base backup client as auxiliary backend process
Next
From: Dent John
Date:
Subject: Re: [WIP] UNNEST(REFCURSOR): allowing SELECT to consume data from aREFCURSOR