Date: Fri, 17 Nov 2017 09:58:11 -0800
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-committers@postgresql.org,
	Thomas Munro <thomas.munro@enterprisedb.com>
Subject: Re: pgsql: Remove BufFile's isTemp flag.
Message-ID: <20171117175811.qgrs75pgyhl42kgr@alap3.anarazel.de>
References: <E1eFVyl-0008J1-RO@gemulon.postgresql.org>
 <23339.1510887494@sss.pgh.pa.us>
 <20171117031149.hslkxis732hkjuog@alap3.anarazel.de>
 <24110.1510888651@sss.pgh.pa.us>
 <32216.1510935834@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <32216.1510935834@sss.pgh.pa.us>
Precedence: bulk

Hi,

On 2017-11-17 11:23:54 -0500, Tom Lane wrote:
> OK, after looking through the history, the reason for isTemp = false
> is indeed to allow BufFileCreate() to maintain its old semantics,
> wherein you could attach a BufFile to an already-existing, possibly
> non-temp file.  There have not been any core callers of BufFileCreate()
> in a long time (maybe not since that commit, in fact), but I imagine
> I left it alone for fear that extensions might be using it.  I see though
> that Bruce ifdef'd it out in 20ad43b5, so there aren't any extensions
> using it either.
> 
> We should flat-out remove the function, since this change makes it
> impossible to resurrect with its old semantics.

That sounds reasonable.


> I wonder whether we should then rename BufFileCreateTemp to just
> BufFileCreate, since it's no longer possible to have a BufFile that
> isn't temp.  I suspect that some attention to the comments might be
> needed too.

Thomas?


> Or maybe we should revert 11e264517.  It doesn't seem to be buying
> much to make up for the loss of flexibility.

There's a bunch of work adding new functionality to buffile.c
pending. And having code paths that have been dead for 10+ years around
and maintaining them in working order doesn't seem like a good use of
time.

Greetings,

Andres Freund