В Fri, 27 Sep 2019 17:24:49 +0900
Michael Paquier <mich...@paquier.xyz> пишет:
> > Looks like some good actionable feedback. I've moved this patch to
> > September, and set it to "Waiting on Author".
>
> The patch is in this state for two months now, so I have switched it
> to "returned with feedback". The latest patch does not apply, and it
> would require an update for the new test module dummy_index_am.
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...
--
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..33f770b 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"
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);
