Update buffile.h/.c comments for removal of non-temp option.

Commit 11e264517 removed BufFile's isTemp flag, thereby eliminating
the possibility of resurrecting BufFileCreate().  But it left that
function in place, as well as a bunch of comments describing how things
worked for the non-temp-file case.  At best, that's now a source of
confusion.  So remove the long-since-commented-out function and change
relevant comments.

I (tgl) wanted to rename BufFileCreateTemp() to BufFileCreate(), but
that seems not to be the consensus position, so leave it as-is.

In passing, fix commit f0828b2fc's failure to update BufFileSeek's
comment to match the change of its argument type from long to off_t.
(I think that might actually have been intentional at the time, but
now that 64-bit off_t is nearly universal, it looks anachronistic.)

Thomas Munro and Tom Lane

Discussion: https://postgr.es/m/E1eFVyl-0008J1-RO@gemulon.postgresql.org

diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c
index b527d38b05b0c4ebe51d93ee712aba0313cc9d04..06bf2fadbf17a1ad38c40f0a06e4e038761a7f6b 100644 (file)
--- 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
@@ -20,8 +20,8 @@
  * of opening/closing file descriptors.
  *
  * Note that BufFile structs are allocated with palloc(), and therefore
- * will go away automatically at transaction end.  If the underlying
- * virtual File is made with OpenTemporaryFile, then all resources for
+ * will go away automatically at query/transaction end.  Since the underlying
+ * virtual Files are made with OpenTemporaryFile, all resources for
  * the file are certain to be cleaned up even if processing is aborted
  * by ereport(ERROR).  The data structures required are made in the
  * palloc context that was current when the BufFile was created, and
@@ -45,8 +45,8 @@
 
 /*
  * We break BufFiles into gigabyte-sized segments, regardless of RELSEG_SIZE.
- * The reason is that we'd like large temporary BufFiles to be spread across
- * multiple tablespaces when available.
+ * The reason is that we'd like large BufFiles to be spread across multiple
+ * tablespaces when available.
  */
 #define MAX_PHYSICAL_FILESIZE  0x40000000
 #define BUFFILE_SEG_SIZE               (MAX_PHYSICAL_FILESIZE / BLCKSZ)
@@ -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
  *
@@ -202,7 +187,7 @@ BufFileClose(BufFile *file)
 
        /* flush any unwritten data */
        BufFileFlush(file);
-       /* close the underlying file(s) (with delete if it's a temp file) */
+       /* close and delete the underlying file(s) */
        for (i = 0; i < file->numFiles; i++)
                FileClose(file->files[i]);
        /* release the buffer space */
@@ -225,10 +210,6 @@ BufFileLoadBuffer(BufFile *file)
 
        /*
         * Advance to next component file if necessary and possible.
-        *
-        * This path can only be taken if there is more than one component, so it
-        * won't interfere with reading a non-temp file that is over
-        * MAX_PHYSICAL_FILESIZE.
         */
        if (file->curOffset >= MAX_PHYSICAL_FILESIZE &&
                file->curFile + 1 < file->numFiles)
@@ -298,8 +279,7 @@ BufFileDumpBuffer(BufFile *file)
                }
 
                /*
-                * Enforce per-file size limit only for temp files, else just try to
-                * write as much as asked...
+                * Determine how much we need to write into this file.
                 */
                bytestowrite = file->nbytes - wpos;
                availbytes = MAX_PHYSICAL_FILESIZE - file->curOffset;
@@ -471,8 +451,8 @@ BufFileFlush(BufFile *file)
  * BufFileSeek
  *
  * Like fseek(), except that target position needs two values in order to
- * work when logical filesize exceeds maximum value representable by long.
- * We do not support relative seeks across more than LONG_MAX, however.
+ * work when logical filesize exceeds maximum value representable by off_t.
+ * We do not support relative seeks across more than that, however.
  *
  * Result is 0 if OK, EOF if not.  Logical position is not moved if an
  * impossible seek is attempted.

diff --git a/src/include/storage/buffile.h b/src/include/storage/buffile.h
index fafcb3f0898127daa9e13b6d7914c832ed98506b..640908717d9258cd4849f442b50bde063d48fb54 100644 (file)
--- 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