Hi! I am starting a new thread as commitfest wants new thread for new patch.
This new thread is a follow up to an https://www.postgresql.org/message-id/ 2620882.s52SJui4ql%40x200m thread, where I've been trying to get rid of StdRdOpions structure, and now I've splitted the patch into smaller parts. Read the quote below, to get what this patch is about > I've been thinking about this patch and came to a conclusion that it > would be better to split it to even smaller parts, so they can be > easily reviewed, one by one. May be even leaving most complex > Heap/Toast part for later. > > This is first part of this patch. Here we give each Access Method it's > own option binary representation instead of StdRdOptions. > > I think this change is obvious. Because, first, Access Methods do not > use most of the values defined in StdRdOptions. > > Second this patch make Options structure code unified for all core > Access Methods. Before some AM used StdRdOptions, some AM used it's own > structure, now all AM uses own structures and code is written in the > same style, so it would be more easy to update it in future. > > John Dent, would you join reviewing this part of the patch? I hope it > will be more easy now... And now I have a newer version of the patch, as I forgot to remove vacuum_cleanup_index_scale_factor from StdRdOptions as it was used only in Btree index and now do not used at all. New version fixes it. -- Software Developer: https://www.upwork.com/freelancers/~014a87e140ff02c0da Body-oriented Therapist: https://vk.com/nataraj_rebalancing (Russian)
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index b5072c0..02ee3c9 100644
--- 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"
@@ -1513,8 +1513,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 838ee68..eec2100 100644
--- a/src/backend/access/hash/hashpage.c
+++ b/src/backend/access/hash/hashpage.c
@@ -359,7 +359,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 = (BLCKSZ * HashGetFillFactor(rel) / 100) / 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 3fb92ea..5170634 100644
--- a/src/backend/access/hash/hashutil.c
+++ b/src/backend/access/hash/hashutil.c
@@ -289,7 +289,28 @@ _hash_checkpage(Relation rel, Buffer buf, int flags)
bytea *
hashoptions(Datum reloptions, bool validate)
{
- return default_reloptions(reloptions, validate, RELOPT_KIND_HASH);
+ relopt_value *options;
+ HashRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(HashRelOptions, fillfactor)},
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_HASH,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(HashRelOptions), options, numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(HashRelOptions), options, numoptions,
+ validate, tab, lengthof(tab));
+
+ pfree(options);
+
+ return (bytea *) rdopts;
}
/*
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 4cfd528..b460d13 100644
--- 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;
+ BTRelOptions *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 = (BTRelOptions *) 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 ab19692..f7c2377 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -725,8 +725,9 @@ _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 = (BLCKSZ * (100 - BTGetFillFactor(wstate->index))
+ / 100);
+
/* 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 1c1029b..244f8ae 100644
--- 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 bc855dd..9115e0e 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -2027,7 +2027,31 @@ BTreeShmemInit(void)
bytea *
btoptions(Datum reloptions, bool validate)
{
- return default_reloptions(reloptions, validate, RELOPT_KIND_BTREE);
+ relopt_value *options;
+ BTRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(BTRelOptions, fillfactor)},
+ {"vacuum_cleanup_index_scale_factor", RELOPT_TYPE_REAL,
+ offsetof(BTRelOptions, vacuum_cleanup_index_scale_factor)}
+
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_BTREE,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(BTRelOptions), options, numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(BTRelOptions), options, numoptions,
+ validate, tab, lengthof(tab));
+
+ pfree(options);
+
+ return (bytea *) rdopts;
}
/*
diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c
index 45472db..99dee1d 100644
--- 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 += (BLCKSZ * (100 - SpGistGetFillFactor(index)) / 100);
needSpace = Min(needSpace, SPGIST_PAGE_CAPACITY);
/* Get the cache entry for this flags setting */
@@ -586,7 +585,29 @@ SpGistInitMetapage(Page page)
bytea *
spgoptions(Datum reloptions, bool validate)
{
- return default_reloptions(reloptions, validate, RELOPT_KIND_SPGIST);
+ relopt_value *options;
+ SpGistRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(SpGistRelOptions, fillfactor)},
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_SPGIST,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(SpGistRelOptions), options,
+ numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(SpGistRelOptions), options,
+ numoptions, validate, tab, lengthof(tab));
+
+ pfree(options);
+
+ return (bytea *) rdopts;
}
/*
diff --git a/src/include/access/hash.h b/src/include/access/hash.h
index 24af778..dbe8dac 100644
--- a/src/include/access/hash.h
+++ b/src/include/access/hash.h
@@ -263,6 +263,17 @@ typedef struct HashMetaPageData
typedef HashMetaPageData *HashMetaPage;
+typedef struct HashRelOptions
+{
+ int32 varlena_header_; /* varlena header (do not touch directly!) */
+ int fillfactor; /* page fill factor in percent (0..100) */
+} HashRelOptions;
+
+#define HashGetFillFactor(relation) \
+ ((relation)->rd_options ? \
+ ((HashRelOptions *) (relation)->rd_options)->fillfactor : \
+ HASH_DEFAULT_FILLFACTOR)
+
/*
* 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 4a80e84..573a44e 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -680,6 +680,19 @@ 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 BTRelOptions
+{
+ 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;
+} BTRelOptions;
+
+#define BTGetFillFactor(relation) \
+ ((relation)->rd_options ? \
+ ((BTRelOptions *) (relation)->rd_options)->fillfactor : \
+ BTREE_DEFAULT_FILLFACTOR)
+
/*
* 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 d787ab2..d5fd7bc 100644
--- 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 428e54b..1031de3 100644
--- a/src/include/access/spgist_private.h
+++ b/src/include/access/spgist_private.h
@@ -22,6 +22,17 @@
#include "utils/relcache.h"
+typedef struct SpGistRelOptions
+{
+ int32 varlena_header_; /* varlena header (do not touch directly!) */
+ int fillfactor; /* page fill factor in percent (0..100) */
+} SpGistRelOptions;
+
+#define SpGistGetFillFactor(relation) \
+ ((relation)->rd_options ? \
+ ((SpGistRelOptions *) (relation)->rd_options)->fillfactor : \
+ SPGIST_DEFAULT_FILLFACTOR)
+
/* Page numbers of fixed-location pages */
#define SPGIST_METAPAGE_BLKNO (0) /* metapage */
#define SPGIST_ROOT_BLKNO (1) /* root for normal entries */
@@ -423,6 +434,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 a5cf804..adce6bde 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -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 */
signature.asc
Description: This is a digitally signed message part.
