Refactor reloption handling for index AMs in-core

This reworks the reloption parsing and build of a couple of index AMs by
creating new structures for each index AM's options.  This split was
already done for BRIN, GIN and GiST (which actually has a fillfactor
parameter), but not for hash, B-tree and SPGiST which relied on
StdRdOptions due to an overlap with the default option set.

This saves a couple of bytes for rd_options in each relcache entry with
indexes making use of relation options, and brings more consistency
between all index AMs.  While on it, add a couple of AssertMacro() calls
to make sure that utility macros to grab values of reloptions are used
with the expected index AM.

Author: Nikolay Shaplov
Reviewed-by: Amit Langote, Michael Paquier, Álvaro Herrera, Dent John
Discussion: https://postgr.es/m/4127670.gFlpRb6XCm@x200m

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 3f22a6c354f1a4dad59660a3b2ec37684c7e0e3f..48377ace24c2db6223ca5c1251e948d0402369fe 100644 (file)
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -23,7 +23,7 @@
 #include "access/htup_details.h"
 #include "access/nbtree.h"
 #include "access/reloptions.h"
-#include "access/spgist.h"
+#include "access/spgist_private.h"
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
 #include "commands/tablespace.h"
@@ -1521,8 +1521,6 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
                offsetof(StdRdOptions, user_catalog_table)},
                {"parallel_workers", RELOPT_TYPE_INT,
                offsetof(StdRdOptions, parallel_workers)},
-               {"vacuum_cleanup_index_scale_factor", RELOPT_TYPE_REAL,
-               offsetof(StdRdOptions, vacuum_cleanup_index_scale_factor)},
                {"vacuum_index_cleanup", RELOPT_TYPE_BOOL,
                offsetof(StdRdOptions, vacuum_index_cleanup)},
                {"vacuum_truncate", RELOPT_TYPE_BOOL,

diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c
index 63697bfeb568a9f5e82a158431fdc4598a7d6cf8..f84cee893581fc5c02243d4344668d5aaad3c4b3 100644 (file)
--- a/src/backend/access/hash/hashpage.c
+++ b/src/backend/access/hash/hashpage.c
@@ -358,7 +358,7 @@ _hash_init(Relation rel, double num_tuples, ForkNumber forkNum)
        data_width = sizeof(uint32);
        item_width = MAXALIGN(sizeof(IndexTupleData)) + MAXALIGN(data_width) +
                sizeof(ItemIdData);             /* include the line pointer */
-       ffactor = RelationGetTargetPageUsage(rel, HASH_DEFAULT_FILLFACTOR) / item_width;
+       ffactor = HashGetTargetPageUsage(rel) / item_width;
        /* keep to a sane range */
        if (ffactor < 10)
                ffactor = 10;

diff --git a/src/backend/access/hash/hashutil.c b/src/backend/access/hash/hashutil.c
index c5005f4754036c5fda720b3f5c9283a0fd5bfc24..669dccc492600fed6f22da5813b5f95af455f4e2 100644 (file)
--- a/src/backend/access/hash/hashutil.c
+++ b/src/backend/access/hash/hashutil.c
@@ -289,7 +289,14 @@ _hash_checkpage(Relation rel, Buffer buf, int flags)
 bytea *
 hashoptions(Datum reloptions, bool validate)
 {
-       return default_reloptions(reloptions, validate, RELOPT_KIND_HASH);
+       static const relopt_parse_elt tab[] = {
+               {"fillfactor", RELOPT_TYPE_INT, offsetof(HashOptions, fillfactor)},
+       };
+
+       return (bytea *) build_reloptions(reloptions, validate,
+                                                                         RELOPT_KIND_HASH,
+                                                                         sizeof(HashOptions),
+                                                                         tab, lengthof(tab));
 }
 
 /*

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 4cfd5289ad778ffe6cda00617c787e280ae5447c..c67235ab8071d37b58638ffe90dd80e0a6f1af4b 100644 (file)
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -816,7 +816,7 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
        }
        else
        {
-               StdRdOptions *relopts;
+               BTOptions  *relopts;
                float8          cleanup_scale_factor;
                float8          prev_num_heap_tuples;
 
@@ -827,7 +827,7 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
                 * tuples exceeds vacuum_cleanup_index_scale_factor fraction of
                 * original tuples count.
                 */
-               relopts = (StdRdOptions *) info->index->rd_options;
+               relopts = (BTOptions *) info->index->rd_options;
                cleanup_scale_factor = (relopts &&
                                                                relopts->vacuum_cleanup_index_scale_factor >= 0)
                        ? relopts->vacuum_cleanup_index_scale_factor

diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index fc7d43a0f310660fcc920abba1eb936bef71e1e0..1dd39a95350bb083f0e5be3003c06f5aa106525d 100644 (file)
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -716,8 +716,8 @@ _bt_pagestate(BTWriteState *wstate, uint32 level)
        if (level > 0)
                state->btps_full = (BLCKSZ * (100 - BTREE_NONLEAF_FILLFACTOR) / 100);
        else
-               state->btps_full = RelationGetTargetPageFreeSpace(wstate->index,
-                                                                                                                 BTREE_DEFAULT_FILLFACTOR);
+               state->btps_full = BTGetTargetPageFreeSpace(wstate->index);
+
        /* no parent level, yet */
        state->btps_next = NULL;
 

diff --git a/src/backend/access/nbtree/nbtsplitloc.c b/src/backend/access/nbtree/nbtsplitloc.c
index a04d4e25d69e4bbeeee006f5226d6a92ef9e37a9..29167f1ef58f4fc7572114daa69471bce21c588b 100644 (file)
--- a/src/backend/access/nbtree/nbtsplitloc.c
+++ b/src/backend/access/nbtree/nbtsplitloc.c
@@ -167,7 +167,7 @@ _bt_findsplitloc(Relation rel,
 
        /* Count up total space in data items before actually scanning 'em */
        olddataitemstotal = rightspace - (int) PageGetExactFreeSpace(page);
-       leaffillfactor = RelationGetFillFactor(rel, BTREE_DEFAULT_FILLFACTOR);
+       leaffillfactor = BTGetFillFactor(rel);
 
        /* Passed-in newitemsz is MAXALIGNED but does not include line pointer */
        newitemsz += sizeof(ItemIdData);

diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index 7669a1a66f5a73891f3a7906ef4469d24ea6351a..ee972a146585858e17f2c0da87ee9e600adc06bf 100644 (file)
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -2014,7 +2014,18 @@ BTreeShmemInit(void)
 bytea *
 btoptions(Datum reloptions, bool validate)
 {
-       return default_reloptions(reloptions, validate, RELOPT_KIND_BTREE);
+       static const relopt_parse_elt tab[] = {
+               {"fillfactor", RELOPT_TYPE_INT, offsetof(BTOptions, fillfactor)},
+               {"vacuum_cleanup_index_scale_factor", RELOPT_TYPE_REAL,
+               offsetof(BTOptions, vacuum_cleanup_index_scale_factor)}
+
+       };
+
+       return (bytea *) build_reloptions(reloptions, validate,
+                                                                         RELOPT_KIND_BTREE,
+                                                                         sizeof(BTOptions),
+                                                                         tab, lengthof(tab));
+
 }
 
 /*

diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c
index 45472db147b1bde49f4a5254709238740258ff7a..e2d391ee7540d8a9e5cf1fff1d0787d9d33e9a44 100644 (file)
--- a/src/backend/access/spgist/spgutils.c
+++ b/src/backend/access/spgist/spgutils.c
@@ -408,8 +408,7 @@ SpGistGetBuffer(Relation index, int flags, int needSpace, bool *isNew)
         * related to the ones already on it.  But fillfactor mustn't cause an
         * error for requests that would otherwise be legal.
         */
-       needSpace += RelationGetTargetPageFreeSpace(index,
-                                                                                               SPGIST_DEFAULT_FILLFACTOR);
+       needSpace += SpGistGetTargetPageFreeSpace(index);
        needSpace = Min(needSpace, SPGIST_PAGE_CAPACITY);
 
        /* Get the cache entry for this flags setting */
@@ -586,7 +585,15 @@ SpGistInitMetapage(Page page)
 bytea *
 spgoptions(Datum reloptions, bool validate)
 {
-       return default_reloptions(reloptions, validate, RELOPT_KIND_SPGIST);
+       static const relopt_parse_elt tab[] = {
+               {"fillfactor", RELOPT_TYPE_INT, offsetof(SpGistOptions, fillfactor)},
+       };
+
+       return (bytea *) build_reloptions(reloptions, validate,
+                                                                         RELOPT_KIND_SPGIST,
+                                                                         sizeof(SpGistOptions),
+                                                                         tab, lengthof(tab));
+
 }
 
 /*

diff --git a/src/include/access/brin.h b/src/include/access/brin.h
index fb351b36e03a4b35bff30b056f811beb95ed1410..794088867acf7db4fc51218f171086e7c1508126 100644 (file)
--- a/src/include/access/brin.h
+++ b/src/include/access/brin.h
@@ -37,11 +37,15 @@ typedef struct BrinStatsData
 
 #define BRIN_DEFAULT_PAGES_PER_RANGE   128
 #define BrinGetPagesPerRange(relation) \
-       ((relation)->rd_options ? \
+       (AssertMacro(relation->rd_rel->relkind == RELKIND_INDEX && \
+                                relation->rd_rel->relam == BRIN_AM_OID), \
+        (relation)->rd_options ? \
         ((BrinOptions *) (relation)->rd_options)->pagesPerRange : \
          BRIN_DEFAULT_PAGES_PER_RANGE)
 #define BrinGetAutoSummarize(relation) \
-       ((relation)->rd_options ? \
+       (AssertMacro(relation->rd_rel->relkind == RELKIND_INDEX && \
+                                relation->rd_rel->relam == BRIN_AM_OID), \
+        (relation)->rd_options ? \
         ((BrinOptions *) (relation)->rd_options)->autosummarize : \
          false)
 

diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index 78fcd826f185eab758785ce184ee26b137657394..4e45552b3f2dce8dfceb23aacf6e0f50b8ba234f 100644 (file)
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -14,6 +14,7 @@
 #include "access/gin.h"
 #include "access/ginblock.h"
 #include "access/itup.h"
+#include "catalog/pg_am_d.h"
 #include "fmgr.h"
 #include "storage/bufmgr.h"
 #include "lib/rbtree.h"
@@ -30,10 +31,14 @@ typedef struct GinOptions
 
 #define GIN_DEFAULT_USE_FASTUPDATE     true
 #define GinGetUseFastUpdate(relation) \
-       ((relation)->rd_options ? \
+       (AssertMacro(relation->rd_rel->relkind == RELKIND_INDEX && \
+                                relation->rd_rel->relam == GIN_AM_OID), \
+        (relation)->rd_options ? \
         ((GinOptions *) (relation)->rd_options)->useFastUpdate : GIN_DEFAULT_USE_FASTUPDATE)
 #define GinGetPendingListCleanupSize(relation) \
-       ((relation)->rd_options && \
+       (AssertMacro(relation->rd_rel->relkind == RELKIND_INDEX && \
+                                relation->rd_rel->relam == GIN_AM_OID), \
+        (relation)->rd_options && \
         ((GinOptions *) (relation)->rd_options)->pendingListCleanupSize != -1 ? \
         ((GinOptions *) (relation)->rd_options)->pendingListCleanupSize : \
         gin_pending_list_limit)

diff --git a/src/include/access/hash.h b/src/include/access/hash.h
index 24af778fdf8e55290e05cfd949b0e496d130eaca..ba58cb31bb80a1a1813c105a5770b7927c107a94 100644 (file)
--- a/src/include/access/hash.h
+++ b/src/include/access/hash.h
@@ -20,6 +20,7 @@
 #include "access/amapi.h"
 #include "access/itup.h"
 #include "access/sdir.h"
+#include "catalog/pg_am_d.h"
 #include "lib/stringinfo.h"
 #include "storage/bufmgr.h"
 #include "storage/lockdefs.h"
@@ -263,6 +264,21 @@ typedef struct HashMetaPageData
 
 typedef HashMetaPageData *HashMetaPage;
 
+typedef struct HashOptions
+{
+       int32           varlena_header_;        /* varlena header (do not touch directly!) */
+       int                     fillfactor;             /* page fill factor in percent (0..100) */
+} HashOptions;
+
+#define HashGetFillFactor(relation) \
+       (AssertMacro(relation->rd_rel->relkind == RELKIND_INDEX && \
+                                relation->rd_rel->relam == HASH_AM_OID), \
+        (relation)->rd_options ? \
+        ((HashOptions *) (relation)->rd_options)->fillfactor : \
+        HASH_DEFAULT_FILLFACTOR)
+#define HashGetTargetPageUsage(relation) \
+       (BLCKSZ * HashGetFillFactor(relation) / 100)
+
 /*
  * Maximum size of a hash index item (it's okay to have only one per page)
  */

diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index 4a80e84aa748d7d3d848608e89ce67f7e1584add..18a2a3e71cdce7b83e65dcdfd0bcc29bde325b45 100644 (file)
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -18,6 +18,7 @@
 #include "access/itup.h"
 #include "access/sdir.h"
 #include "access/xlogreader.h"
+#include "catalog/pg_am_d.h"
 #include "catalog/pg_index.h"
 #include "lib/stringinfo.h"
 #include "storage/bufmgr.h"
@@ -680,6 +681,23 @@ typedef BTScanOpaqueData *BTScanOpaque;
 #define SK_BT_DESC                     (INDOPTION_DESC << SK_BT_INDOPTION_SHIFT)
 #define SK_BT_NULLS_FIRST      (INDOPTION_NULLS_FIRST << SK_BT_INDOPTION_SHIFT)
 
+typedef struct BTOptions
+{
+       int32           varlena_header_;        /* varlena header (do not touch directly!) */
+       int                     fillfactor;             /* page fill factor in percent (0..100) */
+       /* fraction of newly inserted tuples prior to trigger index cleanup */
+       float8          vacuum_cleanup_index_scale_factor;
+} BTOptions;
+
+#define BTGetFillFactor(relation) \
+       (AssertMacro(relation->rd_rel->relkind == RELKIND_INDEX && \
+                                relation->rd_rel->relam == BTREE_AM_OID), \
+        (relation)->rd_options ? \
+        ((BTOptions *) (relation)->rd_options)->fillfactor : \
+        BTREE_DEFAULT_FILLFACTOR)
+#define BTGetTargetPageFreeSpace(relation) \
+       (BLCKSZ * (100 - BTGetFillFactor(relation)) / 100)
+
 /*
  * Constant definition for progress reporting.  Phase numbers must match
  * btbuildphasename.

diff --git a/src/include/access/spgist.h b/src/include/access/spgist.h
index d787ab213dc13db2ef73d479c6305e91e240fb62..d5fd7bcc02b60e559efe18f08b0d8424ae9cb7aa 100644 (file)
--- a/src/include/access/spgist.h
+++ b/src/include/access/spgist.h
@@ -19,10 +19,6 @@
 #include "lib/stringinfo.h"
 
 
-/* reloption parameters */
-#define SPGIST_MIN_FILLFACTOR                  10
-#define SPGIST_DEFAULT_FILLFACTOR              80
-
 /* SPGiST opclass support function numbers */
 #define SPGIST_CONFIG_PROC                             1
 #define SPGIST_CHOOSE_PROC                             2

diff --git a/src/include/access/spgist_private.h b/src/include/access/spgist_private.h
index 428e54bfbd2fc4b4c11e9bd60173175250b1de5e..46c6cb0ce298f21413d31898fbbf4369b6c2ea74 100644 (file)
--- a/src/include/access/spgist_private.h
+++ b/src/include/access/spgist_private.h
@@ -16,12 +16,29 @@
 
 #include "access/itup.h"
 #include "access/spgist.h"
+#include "catalog/pg_am_d.h"
 #include "nodes/tidbitmap.h"
 #include "storage/buf.h"
 #include "utils/geo_decls.h"
 #include "utils/relcache.h"
 
 
+typedef struct SpGistOptions
+{
+       int32           varlena_header_;        /* varlena header (do not touch directly!) */
+       int                     fillfactor;             /* page fill factor in percent (0..100) */
+} SpGistOptions;
+
+#define SpGistGetFillFactor(relation) \
+       (AssertMacro(relation->rd_rel->relkind == RELKIND_INDEX && \
+                                relation->rd_rel->relam == SPGIST_AM_OID), \
+        (relation)->rd_options ? \
+        ((SpGistOptions *) (relation)->rd_options)->fillfactor : \
+        SPGIST_DEFAULT_FILLFACTOR)
+#define SpGistGetTargetPageFreeSpace(relation) \
+       (BLCKSZ * (100 - SpGistGetFillFactor(relation)) / 100)
+
+
 /* Page numbers of fixed-location pages */
 #define SPGIST_METAPAGE_BLKNO   (0)    /* metapage */
 #define SPGIST_ROOT_BLKNO               (1)    /* root for normal entries */
@@ -423,6 +440,11 @@ typedef SpGistDeadTupleData *SpGistDeadTuple;
 #define GBUF_REQ_NULLS(flags)  ((flags) & GBUF_NULLS)
 
 /* spgutils.c */
+
+/* reloption parameters */
+#define SPGIST_MIN_FILLFACTOR                  10
+#define SPGIST_DEFAULT_FILLFACTOR              80
+
 extern SpGistCache *spgGetCache(Relation index);
 extern void initSpGistState(SpGistState *state, Relation index);
 extern Buffer SpGistNewBuffer(Relation index);

diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 8b8b237f0d828fd160a9cbf9e2175389d3c2e134..31d8a1a10e7657afca940bdaa9c9bd0745ab94d7 100644 (file)
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -235,7 +235,7 @@ typedef struct ForeignKeyCacheInfo
 
 /*
  * StdRdOptions
- *             Standard contents of rd_options for heaps and generic indexes.
+ *             Standard contents of rd_options for heaps.
  *
  * RelationGetFillFactor() and RelationGetTargetPageFreeSpace() can only
  * be applied to relations that use this format or a superset for
@@ -265,7 +265,6 @@ typedef struct StdRdOptions
        int32           vl_len_;                /* varlena header (do not touch directly!) */
        int                     fillfactor;             /* page fill factor in percent (0..100) */
        /* fraction of newly inserted tuples prior to trigger index cleanup */
-       float8          vacuum_cleanup_index_scale_factor;
        int                     toast_tuple_target; /* target for tuple toasting */
        AutoVacOpts autovacuum;         /* autovacuum-related options */
        bool            user_catalog_table; /* use as an additional catalog relation */

diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 60c76cb16049fb9afaa16fa0836e4c544d05c06f..673338b53683c89943fc6f512910b78e3f56dea4 100644 (file)
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -171,6 +171,7 @@ BTInsertStateData
 BTLeader
 BTMetaPageData
 BTOneVacInfo
+BTOptions
 BTPS_State
 BTPageOpaque
 BTPageOpaqueData
@@ -978,6 +979,7 @@ HashJoinTuple
 HashMemoryChunk
 HashMetaPage
 HashMetaPageData
+HashOptions
 HashPageOpaque
 HashPageOpaqueData
 HashPageStat
@@ -2270,6 +2272,7 @@ SpGistLeafTupleData
 SpGistMetaPageData
 SpGistNodeTuple
 SpGistNodeTupleData
+SpGistOptions
 SpGistPageOpaque
 SpGistPageOpaqueData
 SpGistScanOpaque