Date: Fri, 20 Jul 2018 11:14:59 +0300
From: Oskari Saarenmaa <os@ohmu.fi>
To: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: pgsql-hackers@postgresql.org,
	Tobias Oberstein <tobias.oberstein@gmail.com>
Subject: Re: pread() and pwrite()
Message-ID: <20180720081459.GC22214@saarenmaa.fi>
References: 
 <CAEepm=02rapCpPR3ZGF2vW=SBHSdFYO_bz_f-wwWJonmA3APgw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAEepm=02rapCpPR3ZGF2vW=SBHSdFYO_bz_f-wwWJonmA3APgw@mail.gmail.com>
User-Agent: Mutt/1.5.20 (2009-12-10)
Precedence: bulk

On Thu, Jul 12, 2018 at 01:55:31PM +1200, Thomas Munro wrote:
> A couple of years ago, Oskari Saarenmaa proposed a patch[1] to adopt
> $SUBJECT.  Last year, Tobias Oberstein argued again that we should do
> that[2].  At the end of that thread there was a +1 from multiple
> committers in support of getting it done for PostgreSQL 12.  Since
> Oskari hasn't returned, I decided to dust off his patch and start a
> new thread.

Thanks for picking this up - I was meaning to get back to this, but have
unfortunately been too busy with other projects.

> 1.  FileRead() and FileWrite() are replaced with FileReadAt() and
> FileWriteAt(), taking a new offset argument.  Now we can do one
> syscall instead of two for random reads and writes.

> [...]  I'm not even sure I'd bother adding "At" to the
> function names.  If there are any extensions that want the old
> interface they will fail to compile either way.  Note that the BufFile
> interface remains the same, so hopefully that covers many use cases.

IIRC I used the "At" suffixes in my first version of the patch before
completely removing the functions which didn't take an offset argument
Now that they're gone I agree that we could just drop the "At" suffix;
"at" suffix is also used by various POSIX functions to operate in a
specific directory which may just add to confusion.

> I guess the only remaining reason to use FileSeek() is to get the file
> size?  So I wonder why SEEK_SET remains valid in the patch... if my
> suspicion is correct that only SEEK_END still has a reason to exist,
> perhaps we should just kill FileSeek() and add FileSize() or something
> instead?

I see you did this in your updated patch :+1:

Happy to see this patch move forward.

/ Oskari