While working on patch for attribute options for indexes (see
http://www.postgresql.org/message-id/5213596.TqFRiqmCTe@nataraj-amd64 )
I found out that code for reloptions is not flexible at all.
All definitions of attoptons are stored in central catalog in
src/backend/access/common/reloptions.c
It is done this way for both heap and tuple relations, and also for all index
access methods that goes with source code.
Most of the code of the indexes is now hidden behind
"access method" abstraction, but not the reloption code. If you grep through
src/backend/access/common/reloptions.c, you will find RELOPT_KIND_GIN,
RELOPT_KIND_BTREE, RELOPT_KIND_GIST, RELOPT_KIND_SPGIST, RELOPT_KIND_BRIN...
This all should me moved behind "access method" abstraction...
Custom indexes, like postgresql/contrib/bloom can add own reloptions, and
relopt_kind. But the number of relopt_kinfd available is short (it would be
enough for reloptions, but if we add attoptions, we will meet shortage soon).
So my proposial is the following: Each access method has it's own catalog of
options (reloptions, and later attoptions) and when it want to call any
function from src/backend/access/common/reloptions.c it uses a reference to
that catalog instead of relopt_kind.
In the patch that is attached to this message, there is an idea of how it can
be done.
In that patch I've left relopt_kind, and added refence to options catalog,
and functions from reloptions.c uses that one that is defined. I've implemented
"catalog reference" version for bloom index, and all the rest still work as
they were.
In final patch there should be no relopt_kind at all, all descriptions of
reloptions for indexes should go to src/backend/access/[am-name], reloptions
for heap, toast, views and so on, should stay in
src/backend/access/common/reloptions.c but should be stored as separate option
catalog for each type. I still not sure what to do with
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST options. But one or another solutions can
be found...
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c
index 05dbe87..a71f7d0 100644
--- a/contrib/bloom/blutils.c
+++ b/contrib/bloom/blutils.c
@@ -36,8 +36,8 @@
PG_FUNCTION_INFO_V1(blhandler);
-/* Kind of relation optioms for bloom index */
-static relopt_kind bl_relopt_kind;
+/* Catalog of relation options for bloom index */
+static options_catalog bl_relopt_catalog;
static int32 myRand(void);
static void mySrand(uint32 seed);
@@ -51,15 +51,18 @@ _PG_init(void)
int i;
char buf[16];
- bl_relopt_kind = add_reloption_kind();
+ bl_relopt_catalog.definitions = NULL;
+ bl_relopt_catalog.num = 0;
+ bl_relopt_catalog.max = 0;
+ bl_relopt_catalog.need_initialization = 1;
- add_int_reloption(bl_relopt_kind, "length",
+ add_int_reloption(0, &bl_relopt_catalog, "length",
"Length of signature in uint16 type", 5, 1, 256);
for (i = 0; i < INDEX_MAX_KEYS; i++)
{
snprintf(buf, 16, "col%d", i + 1);
- add_int_reloption(bl_relopt_kind, buf,
+ add_int_reloption(0, &bl_relopt_catalog, buf,
"Number of bits for corresponding column", 2, 1, 2048);
}
}
@@ -457,7 +460,7 @@ bloptions(Datum reloptions, bool validate)
tab[i + 1].offset = offsetof(BloomOptions, bitSize[i]);
}
- options = parseRelOptions(reloptions, validate, bl_relopt_kind, &numoptions);
+ options = parseRelOptions(reloptions, validate, &bl_relopt_catalog, 0, &numoptions);
rdopts = allocateReloptStruct(sizeof(BloomOptions), options, numoptions);
fillRelOptions((void *) rdopts, sizeof(BloomOptions), options, numoptions,
validate, tab, INDEX_MAX_KEYS + 1);
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 89bad05..c804212 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -761,7 +761,7 @@ brinoptions(Datum reloptions, bool validate)
{"pages_per_range", RELOPT_TYPE_INT, offsetof(BrinOptions, pagesPerRange)}
};
- options = parseRelOptions(reloptions, validate, RELOPT_KIND_BRIN,
+ options = parseRelOptions(reloptions, validate, NULL, RELOPT_KIND_BRIN,
&numoptions);
/* if none set, we're done */
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 7448c7f..38fe2c8 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -378,6 +378,7 @@ static bits32 last_assigned_kind = RELOPT_KIND_LAST_DEFAULT;
static int num_custom_options = 0;
static relopt_gen **custom_options = NULL;
static bool need_initialization = true;
+static int max_custom_options = 0;
static void initialize_reloptions(void);
static void parse_one_reloption(relopt_value *option, char *text_str,
@@ -496,32 +497,31 @@ add_reloption_kind(void)
* main parser table.
*/
static void
-add_reloption(relopt_gen *newoption)
+add_reloption(relopt_gen *newoption, options_catalog * catalog)
{
- static int max_custom_options = 0;
-
- if (num_custom_options >= max_custom_options)
+ if (catalog->num + 1 >= catalog->max)
{
MemoryContext oldcxt;
oldcxt = MemoryContextSwitchTo(TopMemoryContext);
- if (max_custom_options == 0)
+ if (catalog->max == 0)
{
- max_custom_options = 8;
- custom_options = palloc(max_custom_options * sizeof(relopt_gen *));
+ catalog->max = 8;
+ catalog->definitions = palloc(catalog->max * sizeof(relopt_gen *));
}
else
{
- max_custom_options *= 2;
- custom_options = repalloc(custom_options,
- max_custom_options * sizeof(relopt_gen *));
+ catalog->max *= 2;
+ catalog->definitions = repalloc(catalog->definitions,
+ catalog->max * sizeof(relopt_gen *));
}
MemoryContextSwitchTo(oldcxt);
}
- custom_options[num_custom_options++] = newoption;
-
- need_initialization = true;
+ catalog->definitions[catalog->num] = newoption;
+ catalog->definitions[catalog->num + 1] = NULL;
+ catalog->num++;
+ catalog->need_initialization = true;
}
/*
@@ -578,15 +578,33 @@ allocate_reloption(bits32 kinds, int type, char *name, char *desc)
* Add a new boolean reloption
*/
void
-add_bool_reloption(bits32 kinds, char *name, char *desc, bool default_val)
+add_bool_reloption(bits32 kinds, options_catalog * catalog,
+ char *name, char *desc, bool default_val)
{
relopt_bool *newoption;
newoption = (relopt_bool *) allocate_reloption(kinds, RELOPT_TYPE_BOOL,
name, desc);
newoption->default_val = default_val;
-
- add_reloption((relopt_gen *) newoption);
+
+ if (catalog)
+ {
+ add_reloption((relopt_gen *) newoption, catalog);
+ } else
+ {
+ options_catalog static_catalog;
+ static_catalog.num = num_custom_options;
+ static_catalog.max = max_custom_options;
+ static_catalog.need_initialization = need_initialization;
+ static_catalog.definitions = custom_options;
+
+ add_reloption((relopt_gen *) newoption, &static_catalog);
+
+ num_custom_options = static_catalog.num;
+ max_custom_options = static_catalog.max;
+ need_initialization = static_catalog.need_initialization;
+ custom_options = static_catalog.definitions;
+ }
}
/*
@@ -594,8 +612,8 @@ add_bool_reloption(bits32 kinds, char *name, char *desc, bool default_val)
* Add a new integer reloption
*/
void
-add_int_reloption(bits32 kinds, char *name, char *desc, int default_val,
- int min_val, int max_val)
+add_int_reloption(bits32 kinds, options_catalog * catalog,
+ char *name, char *desc, int default_val, int min_val, int max_val)
{
relopt_int *newoption;
@@ -605,7 +623,24 @@ add_int_reloption(bits32 kinds, char *name, char *desc, int default_val,
newoption->min = min_val;
newoption->max = max_val;
- add_reloption((relopt_gen *) newoption);
+ if (catalog)
+ {
+ add_reloption((relopt_gen *) newoption, catalog);
+ } else
+ {
+ options_catalog static_catalog;
+ static_catalog.num = num_custom_options;
+ static_catalog.max = max_custom_options;
+ static_catalog.need_initialization = need_initialization;
+ static_catalog.definitions = custom_options;
+
+ add_reloption((relopt_gen *) newoption, &static_catalog);
+
+ num_custom_options = static_catalog.num;
+ max_custom_options = static_catalog.max;
+ need_initialization = static_catalog.need_initialization;
+ custom_options = static_catalog.definitions;
+ }
}
/*
@@ -613,8 +648,9 @@ add_int_reloption(bits32 kinds, char *name, char *desc, int default_val,
* Add a new float reloption
*/
void
-add_real_reloption(bits32 kinds, char *name, char *desc, double default_val,
- double min_val, double max_val)
+add_real_reloption(bits32 kinds, options_catalog * catalog,
+ char *name, char *desc, double default_val,
+ double min_val, double max_val)
{
relopt_real *newoption;
@@ -624,7 +660,24 @@ add_real_reloption(bits32 kinds, char *name, char *desc, double default_val,
newoption->min = min_val;
newoption->max = max_val;
- add_reloption((relopt_gen *) newoption);
+ if (catalog)
+ {
+ add_reloption((relopt_gen *) newoption, catalog);
+ } else
+ {
+ options_catalog static_catalog;
+ static_catalog.num = num_custom_options;
+ static_catalog.max = max_custom_options;
+ static_catalog.need_initialization = need_initialization;
+ static_catalog.definitions = custom_options;
+
+ add_reloption((relopt_gen *) newoption, &static_catalog);
+
+ num_custom_options = static_catalog.num;
+ max_custom_options = static_catalog.max;
+ need_initialization = static_catalog.need_initialization;
+ custom_options = static_catalog.definitions;
+ }
}
/*
@@ -637,8 +690,8 @@ add_real_reloption(bits32 kinds, char *name, char *desc, double default_val,
* the validation.
*/
void
-add_string_reloption(bits32 kinds, char *name, char *desc, char *default_val,
- validate_string_relopt validator)
+add_string_reloption(bits32 kinds, options_catalog * catalog, char *name,
+ char *desc, char *default_val, validate_string_relopt validator)
{
relopt_string *newoption;
@@ -663,7 +716,24 @@ add_string_reloption(bits32 kinds, char *name, char *desc, char *default_val,
newoption->default_isnull = true;
}
- add_reloption((relopt_gen *) newoption);
+ if (catalog)
+ {
+ add_reloption((relopt_gen *) newoption, catalog);
+ } else
+ {
+ options_catalog static_catalog;
+ static_catalog.num = num_custom_options;
+ static_catalog.max = max_custom_options;
+ static_catalog.need_initialization = need_initialization;
+ static_catalog.definitions = custom_options;
+
+ add_reloption((relopt_gen *) newoption, &static_catalog);
+
+ num_custom_options = static_catalog.num;
+ max_custom_options = static_catalog.max;
+ need_initialization = static_catalog.need_initialization;
+ custom_options = static_catalog.definitions;
+ }
}
/*
@@ -964,21 +1034,31 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
* be freed by the caller.
*/
relopt_value *
-parseRelOptions(Datum options, bool validate, relopt_kind kind,
- int *numrelopts)
+parseRelOptions(Datum options, bool validate, options_catalog * catalog,
+ relopt_kind kind, int *numrelopts)
{
relopt_value *reloptions;
int numoptions = 0;
int i;
int j;
+ options_catalog static_catalog;
if (need_initialization)
initialize_reloptions();
+ if (! catalog)
+ {
+ static_catalog.definitions = relOpts;
+ catalog = &static_catalog;
+ } else
+ {
+ Assert(kind == 0);
+ }
+
/* Build a list of expected options, based on kind */
- for (i = 0; relOpts[i]; i++)
- if (relOpts[i]->kinds & kind)
+ for (i = 0; catalog->definitions[i]; i++)
+ if (! kind || (catalog->definitions[i]->kinds & kind))
numoptions++;
if (numoptions == 0)
@@ -989,11 +1069,11 @@ parseRelOptions(Datum options, bool validate, relopt_kind kind,
reloptions = palloc(numoptions * sizeof(relopt_value));
- for (i = 0, j = 0; relOpts[i]; i++)
+ for (i = 0, j = 0; catalog->definitions[i]; i++)
{
- if (relOpts[i]->kinds & kind)
+ if (! kind || (catalog->definitions[i]->kinds & kind))
{
- reloptions[j].gen = relOpts[i];
+ reloptions[j].gen = catalog->definitions[i];
reloptions[j].isset = false;
j++;
}
@@ -1305,7 +1385,7 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
offsetof(StdRdOptions, parallel_degree)}
};
- options = parseRelOptions(reloptions, validate, kind, &numoptions);
+ options = parseRelOptions(reloptions, validate, NULL, kind, &numoptions);
/* if none set, we're done */
if (numoptions == 0)
@@ -1337,7 +1417,7 @@ view_reloptions(Datum reloptions, bool validate)
offsetof(ViewOptions, check_option_offset)}
};
- options = parseRelOptions(reloptions, validate, RELOPT_KIND_VIEW, &numoptions);
+ options = parseRelOptions(reloptions, validate, NULL, RELOPT_KIND_VIEW, &numoptions);
/* if none set, we're done */
if (numoptions == 0)
@@ -1417,7 +1497,7 @@ attribute_reloptions(Datum reloptions, bool validate)
{"n_distinct_inherited", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct_inherited)}
};
- options = parseRelOptions(reloptions, validate, RELOPT_KIND_ATTRIBUTE,
+ options = parseRelOptions(reloptions, validate, NULL, RELOPT_KIND_ATTRIBUTE,
&numoptions);
/* if none set, we're done */
@@ -1449,7 +1529,7 @@ tablespace_reloptions(Datum reloptions, bool validate)
{"effective_io_concurrency", RELOPT_TYPE_INT, offsetof(TableSpaceOpts, effective_io_concurrency)}
};
- options = parseRelOptions(reloptions, validate, RELOPT_KIND_TABLESPACE,
+ options = parseRelOptions(reloptions, validate, NULL, RELOPT_KIND_TABLESPACE,
&numoptions);
/* if none set, we're done */
diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c
index a2450f4..5a3d6e0 100644
--- a/src/backend/access/gin/ginutil.c
+++ b/src/backend/access/gin/ginutil.c
@@ -573,7 +573,7 @@ ginoptions(Datum reloptions, bool validate)
pendingListCleanupSize)}
};
- options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIN,
+ options = parseRelOptions(reloptions, validate, NULL, RELOPT_KIND_GIN,
&numoptions);
/* if none set, we're done */
diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c
index fac166d..465e51f 100644
--- a/src/backend/access/gist/gistutil.c
+++ b/src/backend/access/gist/gistutil.c
@@ -819,7 +819,7 @@ gistoptions(Datum reloptions, bool validate)
{"buffering", RELOPT_TYPE_STRING, offsetof(GiSTOptions, bufferingModeOffset)}
};
- options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIST,
+ options = parseRelOptions(reloptions, validate, NULL, RELOPT_KIND_GIST,
&numoptions);
/* if none set, we're done */
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index 469ac67..216f0c7 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -126,6 +126,13 @@ typedef struct
int offset; /* offset of field in result struct */
} relopt_parse_elt;
+typedef struct
+{
+ relopt_gen **definitions;
+ int num;
+ int max;
+ bool need_initialization;
+} options_catalog;
/*
* These macros exist for the convenience of amoptions writers (but consider
@@ -245,14 +252,16 @@ typedef struct
extern relopt_kind add_reloption_kind(void);
-extern void add_bool_reloption(bits32 kinds, char *name, char *desc,
- bool default_val);
-extern void add_int_reloption(bits32 kinds, char *name, char *desc,
- int default_val, int min_val, int max_val);
-extern void add_real_reloption(bits32 kinds, char *name, char *desc,
- double default_val, double min_val, double max_val);
-extern void add_string_reloption(bits32 kinds, char *name, char *desc,
- char *default_val, validate_string_relopt validator);
+extern void add_bool_reloption(bits32 kinds, options_catalog * catalog,
+ char *name, char *desc, bool default_val);
+extern void add_int_reloption(bits32 kinds, options_catalog * catalog,
+ char *name, char *desc, int default_val, int min_val, int max_val);
+extern void add_real_reloption(bits32 kinds, options_catalog * catalog,
+ char *name, char *desc, double default_val,
+ double min_val, double max_val);
+extern void add_string_reloption(bits32 kinds, options_catalog * catalog,
+ char *name, char *desc, char *default_val,
+ validate_string_relopt validator);
extern Datum transformRelOptions(Datum oldOptions, List *defList,
char *namspace, char *validnsps[],
@@ -261,7 +270,7 @@ extern List *untransformRelOptions(Datum options);
extern bytea *extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
amoptions_function amoptions);
extern relopt_value *parseRelOptions(Datum options, bool validate,
- relopt_kind kind, int *numrelopts);
+ options_catalog * catalog, relopt_kind kind, int *numrelopts);
extern void *allocateReloptStruct(Size base, relopt_value *options,
int numoptions);
extern void fillRelOptions(void *rdopts, Size basesize,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
