Revert "Rename routines for write/read of pgstats file"

This reverts commit ed823da1289, that has made pgstat_write_chunk() and
pgstat_read_chunk() available for public use.  These routines do not
have a symmetric API definition across reads and writes, with the write
part returning a void status, deferring an error detection once all the
stats entries have been processed with an ferror(), and the read part
returning a boolean status.

These routines are just tiny wrappers around fread() and fwrite(), and
extensions can just define they own routines instead of relying on the
same facilities as the core pgstat.c.  This commit removes their
declaration from the public headers, to reduce the confusion.
test_custom_stats is updated to use its own read/write routines.
Perhaps something better could be designed in the future; trying to do
so for v19 is not feasable during beta.

Reported-by: Peter Eisentraut <peter@eisentraut.org>
Author: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/a4a8e9af-3eaf-4bbf-9b21-21620f3fc434@eisentraut.org
Backpatch-through: 19

diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 8b2a5ec367503f8290acb6ef3cc3c1dba76f0a55..debcc87af5032836be2f039c6fe707ca94c328a0 100644 (file)
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -1619,15 +1619,18 @@ pgstat_assert_is_up(void)
  * ------------------------------------------------------------
  */
 
-/* helper for pgstat_write_statsfile() */
-void
-pgstat_write_chunk(FILE *fpout, void *ptr, size_t len)
+#define write_chunk_s(fpout, ptr) write_chunk(fpout, ptr, sizeof(*ptr))
+#define read_chunk_s(fpin, ptr) read_chunk(fpin, ptr, sizeof(*ptr))
+
+/* helpers for pgstat_write_statsfile() */
+static void
+write_chunk(FILE *fpout, void *ptr, size_t len)
 {
        int                     rc;
 
        rc = fwrite(ptr, len, 1, fpout);
 
-       /* We check for errors with ferror() when done writing the stats. */
+       /* we'll check for errors with ferror once at the end */
        (void) rc;
 }
 
@@ -1672,7 +1675,7 @@ pgstat_write_statsfile(void)
         * Write the file header --- currently just a format ID.
         */
        format_id = PGSTAT_FILE_FORMAT_ID;
-       pgstat_write_chunk_s(fpout, &format_id);
+       write_chunk_s(fpout, &format_id);
 
        /* Write various stats structs for fixed number of objects */
        for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++)
@@ -1697,8 +1700,8 @@ pgstat_write_statsfile(void)
                        ptr = pgStatLocal.snapshot.custom_data[kind - PGSTAT_KIND_CUSTOM_MIN];
 
                fputc(PGSTAT_FILE_ENTRY_FIXED, fpout);
-               pgstat_write_chunk_s(fpout, &kind);
-               pgstat_write_chunk(fpout, ptr, info->shared_data_len);
+               write_chunk_s(fpout, &kind);
+               write_chunk(fpout, ptr, info->shared_data_len);
        }
 
        /*
@@ -1752,7 +1755,7 @@ pgstat_write_statsfile(void)
                {
                        /* normal stats entry, identified by PgStat_HashKey */
                        fputc(PGSTAT_FILE_ENTRY_HASH, fpout);
-                       pgstat_write_chunk_s(fpout, &ps->key);
+                       write_chunk_s(fpout, &ps->key);
                }
                else
                {
@@ -1762,14 +1765,14 @@ pgstat_write_statsfile(void)
                        kind_info->to_serialized_name(&ps->key, shstats, &name);
 
                        fputc(PGSTAT_FILE_ENTRY_NAME, fpout);
-                       pgstat_write_chunk_s(fpout, &ps->key.kind);
-                       pgstat_write_chunk_s(fpout, &name);
+                       write_chunk_s(fpout, &ps->key.kind);
+                       write_chunk_s(fpout, &name);
                }
 
                /* Write except the header part of the entry */
-               pgstat_write_chunk(fpout,
-                                                  pgstat_get_entry_data(ps->key.kind, shstats),
-                                                  pgstat_get_entry_len(ps->key.kind));
+               write_chunk(fpout,
+                                       pgstat_get_entry_data(ps->key.kind, shstats),
+                                       pgstat_get_entry_len(ps->key.kind));
 
                /* Write more data for the entry, if required */
                if (kind_info->to_serialized_data)
@@ -1780,7 +1783,7 @@ pgstat_write_statsfile(void)
        /*
         * No more output to be done. Close the temp file and replace the old
         * pgstat.stat with it.  The ferror() check replaces testing for error
-        * after each individual fputc or fwrite (in pgstat_write_chunk()) above.
+        * after each individual fputc or fwrite (in write_chunk()) above.
         */
        fputc(PGSTAT_FILE_ENTRY_END, fpout);
 
@@ -1817,9 +1820,9 @@ pgstat_write_statsfile(void)
        }
 }
 
-/* helper for pgstat_read_statsfile() */
-bool
-pgstat_read_chunk(FILE *fpin, void *ptr, size_t len)
+/* helpers for pgstat_read_statsfile() */
+static bool
+read_chunk(FILE *fpin, void *ptr, size_t len)
 {
        return fread(ptr, 1, len, fpin) == len;
 }
@@ -1867,7 +1870,7 @@ pgstat_read_statsfile(void)
        /*
         * Verify it's of the expected format.
         */
-       if (!pgstat_read_chunk_s(fpin, &format_id))
+       if (!read_chunk_s(fpin, &format_id))
        {
                elog(WARNING, "could not read format ID");
                goto error;
@@ -1897,7 +1900,7 @@ pgstat_read_statsfile(void)
                                        char       *ptr;
 
                                        /* entry for fixed-numbered stats */
-                                       if (!pgstat_read_chunk_s(fpin, &kind))
+                                       if (!read_chunk_s(fpin, &kind))
                                        {
                                                elog(WARNING, "could not read stats kind for entry of type %c", t);
                                                goto error;
@@ -1937,7 +1940,7 @@ pgstat_read_statsfile(void)
                                                        info->shared_data_off;
                                        }
 
-                                       if (!pgstat_read_chunk(fpin, ptr, info->shared_data_len))
+                                       if (!read_chunk(fpin, ptr, info->shared_data_len))
                                        {
                                                elog(WARNING, "could not read data of stats kind %u for entry of type %c with size %u",
                                                         kind, t, info->shared_data_len);
@@ -1959,7 +1962,7 @@ pgstat_read_statsfile(void)
                                        if (t == PGSTAT_FILE_ENTRY_HASH)
                                        {
                                                /* normal stats entry, identified by PgStat_HashKey */
-                                               if (!pgstat_read_chunk_s(fpin, &key))
+                                               if (!read_chunk_s(fpin, &key))
                                                {
                                                        elog(WARNING, "could not read key for entry of type %c", t);
                                                        goto error;
@@ -1988,12 +1991,12 @@ pgstat_read_statsfile(void)
                                                PgStat_Kind kind;
                                                NameData        name;
 
-                                               if (!pgstat_read_chunk_s(fpin, &kind))
+                                               if (!read_chunk_s(fpin, &kind))
                                                {
                                                        elog(WARNING, "could not read stats kind for entry of type %c", t);
                                                        goto error;
                                                }
-                                               if (!pgstat_read_chunk_s(fpin, &name))
+                                               if (!read_chunk_s(fpin, &name))
                                                {
                                                        elog(WARNING, "could not read name of stats kind %u for entry of type %c",
                                                                 kind, t);
@@ -2068,9 +2071,9 @@ pgstat_read_statsfile(void)
                                                         key.objid, t);
                                        }
 
-                                       if (!pgstat_read_chunk(fpin,
-                                                                                  pgstat_get_entry_data(key.kind, header),
-                                                                                  pgstat_get_entry_len(key.kind)))
+                                       if (!read_chunk(fpin,
+                                                                       pgstat_get_entry_data(key.kind, header),
+                                                                       pgstat_get_entry_len(key.kind)))
                                        {
                                                elog(WARNING, "could not read data for entry %u/%u/%" PRIu64 " of type %c",
                                                         key.kind, key.dboid,

diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index b3dc3ff7d8bb74683a7feb3997a4b7033e09ef91..02022f19b81909858ac143aa13697f5c18c21674 100644 (file)
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -886,11 +886,6 @@ extern PGDLLIMPORT bool pgstat_report_fixed;
 /* Backend-local stats state */
 extern PGDLLIMPORT PgStat_LocalState pgStatLocal;
 
-/* Helper functions for reading and writing of on-disk stats file */
-extern void pgstat_write_chunk(FILE *fpout, void *ptr, size_t len);
-extern bool pgstat_read_chunk(FILE *fpin, void *ptr, size_t len);
-#define pgstat_read_chunk_s(fpin, ptr) pgstat_read_chunk(fpin, ptr, sizeof(*ptr))
-#define pgstat_write_chunk_s(fpout, ptr) pgstat_write_chunk(fpout, ptr, sizeof(*ptr))
 
 /*
  * Implementation of inline functions declared above.

diff --git a/src/test/modules/test_custom_stats/test_custom_var_stats.c b/src/test/modules/test_custom_stats/test_custom_var_stats.c
index 024dae85a45e7709f651e2928f62a6ab57f6ced5..34d474be604dfd0dbfbbb8d3662a6bcdcf725016 100644 (file)
--- a/src/test/modules/test_custom_stats/test_custom_var_stats.c
+++ b/src/test/modules/test_custom_stats/test_custom_var_stats.c
@@ -25,6 +25,12 @@ PG_MODULE_MAGIC_EXT(
                                        .version = PG_VERSION
 );
 
+/* Local helpers for stats file I/O */
+#define write_chunk(fpout, ptr, len) ((void) fwrite(ptr, len, 1, fpout))
+#define write_chunk_s(fpout, ptr) write_chunk(fpout, ptr, sizeof(*ptr))
+#define read_chunk(fpin, ptr, len) (fread(ptr, 1, len, fpin) == (len))
+#define read_chunk_s(fpin, ptr) read_chunk(fpin, ptr, sizeof(*ptr))
+
 #define TEST_CUSTOM_VAR_MAGIC_NUMBER (0xBEEFBEEF)
 
 /*--------------------------------------------------------------------------
@@ -202,7 +208,7 @@ test_custom_stats_var_to_serialized_data(const PgStat_HashKey *key,
         * First mark the main file with a magic number, keeping a trace that some
         * auxiliary data will exist in the secondary statistics file.
         */
-       pgstat_write_chunk_s(statfile, &magic_number);
+       write_chunk_s(statfile, &magic_number);
 
        /* Open statistics file for writing. */
        if (!fd_description)
@@ -222,14 +228,14 @@ test_custom_stats_var_to_serialized_data(const PgStat_HashKey *key,
        }
 
        /* Write offset to the main data file */
-       pgstat_write_chunk_s(statfile, &fd_description_offset);
+       write_chunk_s(statfile, &fd_description_offset);
 
        /*
         * First write the entry key to the secondary statistics file.  This will
         * be cross-checked with the key read from main stats file at loading
         * time.
         */
-       pgstat_write_chunk_s(fd_description, (PgStat_HashKey *) key);
+       write_chunk_s(fd_description, (PgStat_HashKey *) key);
        fd_description_offset += sizeof(PgStat_HashKey);
 
        if (!custom_stats_description_dsa)
@@ -240,7 +246,7 @@ test_custom_stats_var_to_serialized_data(const PgStat_HashKey *key,
        {
                /* length to description file */
                len = 0;
-               pgstat_write_chunk_s(fd_description, &len);
+               write_chunk_s(fd_description, &len);
                fd_description_offset += sizeof(size_t);
                return;
        }
@@ -252,8 +258,8 @@ test_custom_stats_var_to_serialized_data(const PgStat_HashKey *key,
        description = dsa_get_address(custom_stats_description_dsa,
                                                                  entry->description);
        len = strlen(description) + 1;
-       pgstat_write_chunk_s(fd_description, &len);
-       pgstat_write_chunk(fd_description, description, len);
+       write_chunk_s(fd_description, &len);
+       write_chunk(fd_description, description, len);
 
        /*
         * Update offset for next entry, counting for the length (size_t) of the
@@ -287,7 +293,7 @@ test_custom_stats_var_from_serialized_data(const PgStat_HashKey *key,
        PgStat_HashKey file_key;
 
        /* Check the magic number first, in the main file. */
-       if (!pgstat_read_chunk_s(statfile, &magic_number))
+       if (!read_chunk_s(statfile, &magic_number))
        {
                elog(WARNING, "failed to read magic number from statistics file");
                return false;
@@ -304,7 +310,7 @@ test_custom_stats_var_from_serialized_data(const PgStat_HashKey *key,
         * Read the offset from the main stats file, to be able to read the
         * auxiliary data from the secondary statistics file.
         */
-       if (!pgstat_read_chunk_s(statfile, &offset))
+       if (!read_chunk_s(statfile, &offset))
        {
                elog(WARNING, "failed to read metadata offset from statistics file");
                return false;
@@ -335,7 +341,7 @@ test_custom_stats_var_from_serialized_data(const PgStat_HashKey *key,
        }
 
        /* Read the hash key from the secondary statistics file */
-       if (!pgstat_read_chunk_s(fd_description, &file_key))
+       if (!read_chunk_s(fd_description, &file_key))
        {
                elog(WARNING, "failed to read hash key from file");
                return false;
@@ -355,7 +361,7 @@ test_custom_stats_var_from_serialized_data(const PgStat_HashKey *key,
        entry = (PgStatShared_CustomVarEntry *) header;
 
        /* Read the description length and its data */
-       if (!pgstat_read_chunk_s(fd_description, &len))
+       if (!read_chunk_s(fd_description, &len))
        {
                elog(WARNING, "failed to read metadata length from statistics file");
                return false;
@@ -379,7 +385,7 @@ test_custom_stats_var_from_serialized_data(const PgStat_HashKey *key,
        }
 
        buffer = palloc(len);
-       if (!pgstat_read_chunk(fd_description, buffer, len))
+       if (!read_chunk(fd_description, buffer, len))
        {
                pfree(buffer);
                elog(WARNING, "failed to read description from file");