pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
pgsql: Remove BufFile's isTemp flag.
13+ messages / 4 participants
[nested] [flat]

* pgsql: Remove BufFile's isTemp flag.
@ 2017-11-17 02:02  Andres Freund <andres@anarazel.de>
  0 siblings, 1 reply; 13+ messages in thread

From: Andres Freund @ 2017-11-17 02:02 UTC (permalink / raw)
  To: pgsql-committers

Remove BufFile's isTemp flag.

The isTemp flag controls whether buffile.c chops BufFile data up into
1GB segments on disk.  Since it was badly named and always true, get
rid of it.

Author: Thomas Munro (based on suggestion by Peter Geoghegan)
Reviewed-By: Peter Geoghegan, Andres Freund
Discussion: https://postgr.es/m/CAH2-Wz%3D%2B9Rfqh5UdvdW9rGezdhrMGGH-JL1X9FXXVZdeeGeOJA%40mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/11e264517dff7a911d9e6494de86049cab42cde3

Modified Files
--------------
src/backend/storage/file/buffile.c | 43 ++++++++++++++++----------------------
1 file changed, 18 insertions(+), 25 deletions(-)



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: pgsql: Remove BufFile's isTemp flag.
@ 2017-11-17 02:58  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Andres Freund <andres@anarazel.de>
  0 siblings, 1 reply; 13+ messages in thread

From: Tom Lane @ 2017-11-17 02:58 UTC (permalink / raw)
  To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-committers

Andres Freund <andres@anarazel.de> writes:
> Remove BufFile's isTemp flag.
> The isTemp flag controls whether buffile.c chops BufFile data up into
> 1GB segments on disk.  Since it was badly named and always true, get
> rid of it.

[ squint... ]  That used to have an actual purpose connected to
transaction-abort cleanup, IIRC.  It disturbs me that this seems
to have been lost.

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: pgsql: Remove BufFile's isTemp flag.
@ 2017-11-17 03:11  Andres Freund <andres@anarazel.de>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 13+ messages in thread

From: Andres Freund @ 2017-11-17 03:11 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers

On 2017-11-16 21:58:14 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Remove BufFile's isTemp flag.
> > The isTemp flag controls whether buffile.c chops BufFile data up into
> > 1GB segments on disk.  Since it was badly named and always true, get
> > rid of it.
> 
> [ squint... ]  That used to have an actual purpose connected to
> transaction-abort cleanup, IIRC.  It disturbs me that this seems
> to have been

I've not found any such use, searching through buffile.c's history. I
don't quite see how that flag could've been related to abort cleanup
stuff?  There's been another unused caller of makeBufFile, namely
BufFileCreate, that has been #ifdef'ed out for ages (perhaps we
should've removed that with this commit or a long time ago).  Other than
that there seems to not have been any other caller setting that flag
differently since you created the file in db3c4c3a2d980dcd.

Greetings,

Andres Freund




^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: pgsql: Remove BufFile's isTemp flag.
@ 2017-11-17 03:17  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Andres Freund <andres@anarazel.de>
  0 siblings, 1 reply; 13+ messages in thread

From: Tom Lane @ 2017-11-17 03:17 UTC (permalink / raw)
  To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-committers

Andres Freund <andres@anarazel.de> writes:
> On 2017-11-16 21:58:14 -0500, Tom Lane wrote:
>> [ squint... ]  That used to have an actual purpose connected to
>> transaction-abort cleanup, IIRC.  It disturbs me that this seems
>> to have been lost.

> I've not found any such use, searching through buffile.c's history. I
> don't quite see how that flag could've been related to abort cleanup
> stuff?  There's been another unused caller of makeBufFile, namely
> BufFileCreate, that has been #ifdef'ed out for ages (perhaps we
> should've removed that with this commit or a long time ago).  Other than
> that there seems to not have been any other caller setting that flag
> differently since you created the file in db3c4c3a2d980dcd.

I'm tired for today, but will take a closer look tomorrow.  I do not
think the mechanism was created without a purpose ...

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: pgsql: Remove BufFile's isTemp flag.
@ 2017-11-17 16:23  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 2 replies; 13+ messages in thread

From: Tom Lane @ 2017-11-17 16:23 UTC (permalink / raw)
  To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-committers

I wrote:
> Andres Freund <andres@anarazel.de> writes:
>> On 2017-11-16 21:58:14 -0500, Tom Lane wrote:
>>> [ squint... ]  That used to have an actual purpose connected to
>>> transaction-abort cleanup, IIRC.  It disturbs me that this seems
>>> to have been lost.

>> I've not found any such use, searching through buffile.c's history. I
>> don't quite see how that flag could've been related to abort cleanup
>> stuff?  There's been another unused caller of makeBufFile, namely
>> BufFileCreate, that has been #ifdef'ed out for ages (perhaps we
>> should've removed that with this commit or a long time ago).  Other than
>> that there seems to not have been any other caller setting that flag
>> differently since you created the file in db3c4c3a2d980dcd.

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.  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.

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

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: pgsql: Remove BufFile's isTemp flag.
@ 2017-11-17 17:51  Peter Geoghegan <pg@bowt.ie>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 0 replies; 13+ messages in thread

From: Peter Geoghegan @ 2017-11-17 17:51 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-committers

On Fri, Nov 17, 2017 at 8:23 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> We should flat-out remove the function, since this change makes it
> impossible to resurrect with its old semantics.  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.

+1

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

Thomas wrote code that makes it possible to extend individual BufFiles
with other BufFiles across backends. This code will be used by
parallel CREATE INDEX, though it is something included in recent
versions of his parallel hash join patchset. This process happens at a
higher level than buffile.c, and should get the tricky details of
resource management right.

I think it's likely that this will be committed for v11.

-- 
Peter Geoghegan




^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: pgsql: Remove BufFile's isTemp flag.
@ 2017-11-17 17:58  Andres Freund <andres@anarazel.de>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 2 replies; 13+ messages in thread

From: Andres Freund @ 2017-11-17 17:58 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@postgresql.org, Thomas Munro <thomas.munro@enterprisedb.com>

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




^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: pgsql: Remove BufFile's isTemp flag.
@ 2017-11-17 18:09  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Andres Freund <andres@anarazel.de>
  1 sibling, 0 replies; 13+ messages in thread

From: Tom Lane @ 2017-11-17 18:09 UTC (permalink / raw)
  To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-committers@postgresql.org, Thomas Munro <thomas.munro@enterprisedb.com>

Andres Freund <andres@anarazel.de> writes:
> On 2017-11-17 11:23:54 -0500, Tom Lane wrote:
>> 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.

OK, if there's active work going on in the area, that's a good reason
to simplify.  But let's get rid of all vestiges of the old non-temp
semantics.

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: pgsql: Remove BufFile's isTemp flag.
@ 2017-11-19 21:55  Thomas Munro <thomas.munro@enterprisedb.com>
  parent: Andres Freund <andres@anarazel.de>
  1 sibling, 1 reply; 13+ messages in thread

From: Thomas Munro @ 2017-11-19 21:55 UTC (permalink / raw)
  To: Andres Freund <andres@anarazel.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers

On Sat, Nov 18, 2017 at 6:58 AM, Andres Freund <andres@anarazel.de> wrote:
> 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?

Here's a patch that does those things.  I'm slightly surprised by the
renaming suggestion though, because it means that an extension that
uses BufFile will need to know how to select the v10 and v11 function
name as appropriate.  Would you backpatch redirect support for the new
name to older versions?

-- 
Thomas Munro
http://www.enterprisedb.com

Attachments:

  [application/octet-stream] tidy-buffile.patch (5.2K, ../../CAEepm=38U=A0Ftk3XV=yqHXubiCoG5mr6wxQpgxTnPVo5tJL=A@mail.gmail.com/2-tidy-buffile.patch)
  download | inline diff:
From 1ba0db4d826c0b1a661a52a3e215c11286edc645 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@enterprisedb.com>
Date: Sat, 18 Nov 2017 08:50:32 +1100
Subject: [PATCH] Remove vestiges of non-temporary file support from buffile.c.

Following on from 11e264517dff7a911d9e6494de86049cab42cde3, it seems a good
time to remove some dead code and some comments that alluded to the
possibility of creating non-temporary BufFiles.  Also rename
BufFileCreateTemp() to BufFileCreate().

Author: Thomas Munro, based on feedback from Tom Lane and Andres Freund
Discussion: https://postgr.es/m/32216.1510935834%40sss.pgh.pa.us
---
 src/backend/access/gist/gistbuildbuffers.c |  2 +-
 src/backend/executor/nodeHashjoin.c        |  2 +-
 src/backend/storage/file/buffile.c         | 19 ++-----------------
 src/backend/utils/sort/logtape.c           |  2 +-
 src/backend/utils/sort/tuplestore.c        |  2 +-
 src/include/storage/buffile.h              |  4 ++--
 6 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/src/backend/access/gist/gistbuildbuffers.c b/src/backend/access/gist/gistbuildbuffers.c
index 88cee2028da..205aab09ace 100644
--- a/src/backend/access/gist/gistbuildbuffers.c
+++ b/src/backend/access/gist/gistbuildbuffers.c
@@ -58,7 +58,7 @@ gistInitBuildBuffers(int pagesPerBuffer, int levelStep, int maxLevel)
 	 * Create a temporary file to hold buffer pages that are swapped out of
 	 * memory.
 	 */
-	gfbb->pfile = BufFileCreateTemp(false);
+	gfbb->pfile = BufFileCreate(false);
 	gfbb->nFileBlocks = 0;
 
 	/* Initialize free page management. */
diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c
index ab1632cc13d..d60e538b1ab 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -824,7 +824,7 @@ ExecHashJoinSaveTuple(MinimalTuple tuple, uint32 hashvalue,
 	if (file == NULL)
 	{
 		/* First write to this batch file, so open it. */
-		file = BufFileCreateTemp(false);
+		file = BufFileCreate(false);
 		*fileptr = file;
 	}
 
diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c
index b527d38b05b..bed58148b91 100644
--- a/src/backend/storage/file/buffile.c
+++ b/src/backend/storage/file/buffile.c
@@ -1,7 +1,7 @@
 /*-------------------------------------------------------------------------
  *
  * buffile.c
- *	  Management of large buffered files, primarily temporary files.
+ *	  Management of large buffered temporary files.
  *
  * Portions Copyright (c) 1996-2017, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
@@ -161,7 +161,7 @@ extendBufFile(BufFile *file)
  * transaction boundaries.
  */
 BufFile *
-BufFileCreateTemp(bool interXact)
+BufFileCreate(bool interXact)
 {
 	BufFile    *file;
 	File		pfile;
@@ -175,21 +175,6 @@ BufFileCreateTemp(bool interXact)
 	return file;
 }
 
-#ifdef NOT_USED
-/*
- * Create a BufFile and attach it to an already-opened virtual File.
- *
- * This is comparable to fdopen() in stdio.  This is the only way at present
- * to attach a BufFile to a non-temporary file.  Note that BufFiles created
- * in this way CANNOT be expanded into multiple files.
- */
-BufFile *
-BufFileCreate(File file)
-{
-	return makeBufFile(file);
-}
-#endif
-
 /*
  * Close a BufFile
  *
diff --git a/src/backend/utils/sort/logtape.c b/src/backend/utils/sort/logtape.c
index 5ebb6fb11ab..6911dd7bff2 100644
--- a/src/backend/utils/sort/logtape.c
+++ b/src/backend/utils/sort/logtape.c
@@ -388,7 +388,7 @@ LogicalTapeSetCreate(int ntapes)
 	Assert(ntapes > 0);
 	lts = (LogicalTapeSet *) palloc(offsetof(LogicalTapeSet, tapes) +
 									ntapes * sizeof(LogicalTape));
-	lts->pfile = BufFileCreateTemp(false);
+	lts->pfile = BufFileCreate(false);
 	lts->nBlocksAllocated = 0L;
 	lts->nBlocksWritten = 0L;
 	lts->forgetFreeSpace = false;
diff --git a/src/backend/utils/sort/tuplestore.c b/src/backend/utils/sort/tuplestore.c
index 1977b61fd94..93efd195cba 100644
--- a/src/backend/utils/sort/tuplestore.c
+++ b/src/backend/utils/sort/tuplestore.c
@@ -818,7 +818,7 @@ tuplestore_puttuple_common(Tuplestorestate *state, void *tuple)
 			oldowner = CurrentResourceOwner;
 			CurrentResourceOwner = state->resowner;
 
-			state->myfile = BufFileCreateTemp(state->interXact);
+			state->myfile = BufFileCreate(state->interXact);
 
 			CurrentResourceOwner = oldowner;
 
diff --git a/src/include/storage/buffile.h b/src/include/storage/buffile.h
index fafcb3f0898..b6e4b4b0148 100644
--- a/src/include/storage/buffile.h
+++ b/src/include/storage/buffile.h
@@ -1,7 +1,7 @@
 /*-------------------------------------------------------------------------
  *
  * buffile.h
- *	  Management of large buffered files, primarily temporary files.
+ *	  Management of large buffered temporary files.
  *
  * The BufFile routines provide a partial replacement for stdio atop
  * virtual file descriptors managed by fd.c.  Currently they only support
@@ -34,7 +34,7 @@ typedef struct BufFile BufFile;
  * prototypes for functions in buffile.c
  */
 
-extern BufFile *BufFileCreateTemp(bool interXact);
+extern BufFile *BufFileCreate(bool interXact);
 extern void BufFileClose(BufFile *file);
 extern size_t BufFileRead(BufFile *file, void *ptr, size_t size);
 extern size_t BufFileWrite(BufFile *file, void *ptr, size_t size);
-- 
2.15.0



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: pgsql: Remove BufFile's isTemp flag.
@ 2017-11-19 22:00  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Thomas Munro <thomas.munro@enterprisedb.com>
  0 siblings, 1 reply; 13+ messages in thread

From: Tom Lane @ 2017-11-19 22:00 UTC (permalink / raw)
  To: Thomas Munro <thomas.munro@enterprisedb.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-committers

Thomas Munro <thomas.munro@enterprisedb.com> writes:
>> On 2017-11-17 11:23:54 -0500, Tom Lane wrote:
>>> 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.

> Here's a patch that does those things.  I'm slightly surprised by the
> renaming suggestion though, because it means that an extension that
> uses BufFile will need to know how to select the v10 and v11 function
> name as appropriate.  Would you backpatch redirect support for the new
> name to older versions?

No, but if you're concerned about it, we could maintain API compatibility
for extensions with something like

#define BufFileCreateTemp(interXact) BufFileCreate(interXact)

Typically we expect extensions to provide such workarounds for cases that
concern them ... but since this change is purely cosmetic, maybe it should
be treated differently.

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: pgsql: Remove BufFile's isTemp flag.
@ 2017-11-20 00:06  Andres Freund <andres@anarazel.de>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 2 replies; 13+ messages in thread

From: Andres Freund @ 2017-11-20 00:06 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Thomas Munro <thomas.munro@enterprisedb.com>; pgsql-committers

On 2017-11-19 17:00:36 -0500, Tom Lane wrote:
> Thomas Munro <thomas.munro@enterprisedb.com> writes:
> >> On 2017-11-17 11:23:54 -0500, Tom Lane wrote:
> >>> 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.
> 
> > Here's a patch that does those things.  I'm slightly surprised by the
> > renaming suggestion though, because it means that an extension that
> > uses BufFile will need to know how to select the v10 and v11 function
> > name as appropriate.  Would you backpatch redirect support for the new
> > name to older versions?
> 
> No, but if you're concerned about it, we could maintain API compatibility
> for extensions with something like
> 
> #define BufFileCreateTemp(interXact) BufFileCreate(interXact)

I don't really see a point in doing this renaming in the first
place. It's not like the Temp suffix has become inaccurate. I'd perhaps
not add it in the green field, but I don't see a need to change an
existing function name. If anything it seems confusing because you'd
miss something when trivially searching the history / comparing
branches.

Greetings,

Andres Freund




^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: pgsql: Remove BufFile's isTemp flag.
@ 2017-11-20 00:37  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Andres Freund <andres@anarazel.de>
  1 sibling, 0 replies; 13+ messages in thread

From: Tom Lane @ 2017-11-20 00:37 UTC (permalink / raw)
  To: Andres Freund <andres@anarazel.de>; +Cc: Thomas Munro <thomas.munro@enterprisedb.com>; pgsql-committers

Andres Freund <andres@anarazel.de> writes:
> On 2017-11-19 17:00:36 -0500, Tom Lane wrote:
>> No, but if you're concerned about it, we could maintain API compatibility
>> for extensions with something like
>> #define BufFileCreateTemp(interXact) BufFileCreate(interXact)

> I don't really see a point in doing this renaming in the first
> place. It's not like the Temp suffix has become inaccurate. I'd perhaps
> not add it in the green field, but I don't see a need to change an
> existing function name. If anything it seems confusing because you'd
> miss something when trivially searching the history / comparing
> branches.

Well, that's a fair point about history, but the reason I no longer
want the Temp suffix is that it implies that there's such a thing as
a non-temp BufFile.  I think that's misleading if we've cut off any
vestige of support for it.

Anybody else have an opinion?

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: pgsql: Remove BufFile's isTemp flag.
@ 2017-11-25 18:21  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Andres Freund <andres@anarazel.de>
  1 sibling, 0 replies; 13+ messages in thread

From: Tom Lane @ 2017-11-25 18:21 UTC (permalink / raw)
  To: Andres Freund <andres@anarazel.de>; +Cc: Thomas Munro <thomas.munro@enterprisedb.com>; pgsql-committers

Andres Freund <andres@anarazel.de> writes:
> I don't really see a point in doing this renaming in the first
> place. It's not like the Temp suffix has become inaccurate. I'd perhaps
> not add it in the green field, but I don't see a need to change an
> existing function name. If anything it seems confusing because you'd
> miss something when trivially searching the history / comparing
> branches.

It seems that the vote is 2-1 against renaming that function, so I've
committed Thomas' patch without that and with some additional
comment-smithing.

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 13+ messages in thread


end of thread, other threads:[~2017-11-25 18:21 UTC | newest]

Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-11-17 02:02 pgsql: Remove BufFile's isTemp flag. Andres Freund <andres@anarazel.de>
2017-11-17 02:58 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-11-17 03:11   ` Andres Freund <andres@anarazel.de>
2017-11-17 03:17     ` Tom Lane <tgl@sss.pgh.pa.us>
2017-11-17 16:23       ` Tom Lane <tgl@sss.pgh.pa.us>
2017-11-17 17:51         ` Peter Geoghegan <pg@bowt.ie>
2017-11-17 17:58         ` Andres Freund <andres@anarazel.de>
2017-11-17 18:09           ` Tom Lane <tgl@sss.pgh.pa.us>
2017-11-19 21:55           ` Thomas Munro <thomas.munro@enterprisedb.com>
2017-11-19 22:00             ` Tom Lane <tgl@sss.pgh.pa.us>
2017-11-20 00:06               ` Andres Freund <andres@anarazel.de>
2017-11-20 00:37                 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-11-25 18:21                 ` Tom Lane <tgl@sss.pgh.pa.us>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox