rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Support reloptions of enum type 

All our current in core relation options of type string (not many,
admittedly) behave in reality like enums.  But after seeing an
implementation for enum reloptions, it's clear that strings are messier,
so introduce the new reloption type.  Switch all string options to be
enums instead.

Fortunately we have a recently introduced test module for reloptions, so
we don't lose coverage of string reloptions, which may still be used by
third-party modules.

Authors: Nikolay Shaplov, Álvaro Herrera
Reviewed-by: Nikita Glukhov, Aleksandr Parfenov
Discussion: https://postgr.es/m/43332102.S2V5pIjXRx@x200m
This commit is contained in:
Alvaro Herrera 2019-09-25 15:56:52 -03:00
parent caba97a9d9
commit 773df883e8
12 changed files with 214 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

132
src/backend/access/common/reloptions.c
View File

@ -433,7 +433,25 @@ static relopt_real realRelOpts[] =
{{NULL}}
};
static relopt_string stringRelOpts[] =
/* values from GistOptBufferingMode */
relopt_enum_elt_def gistBufferingOptValues[] =
{
{"auto", GIST_OPTION_BUFFERING_AUTO},
{"on", GIST_OPTION_BUFFERING_ON},
{"off", GIST_OPTION_BUFFERING_OFF},
{(const char *) NULL} /* list terminator */
};
/* values from ViewOptCheckOption */
relopt_enum_elt_def viewCheckOptValues[] =
{
/* no value for NOT_SET */
{"local", VIEW_OPTION_CHECK_OPTION_LOCAL},
{"cascaded", VIEW_OPTION_CHECK_OPTION_CASCADED},
{(const char *) NULL} /* list terminator */
};
static relopt_enum enumRelOpts[] =
{
{
{
@ -442,10 +460,9 @@ static relopt_string stringRelOpts[] =
RELOPT_KIND_GIST,
AccessExclusiveLock
},
4,
false,
gistValidateBufferingOption,
"auto"
gistBufferingOptValues,
GIST_OPTION_BUFFERING_AUTO,
gettext_noop("Valid values are \"on\", \"off\", and \"auto\".")
},
{
{
@ -454,15 +471,20 @@ static relopt_string stringRelOpts[] =
RELOPT_KIND_VIEW,
AccessExclusiveLock
},
0,
true,
validateWithCheckOption,
NULL
viewCheckOptValues,
VIEW_OPTION_CHECK_OPTION_NOT_SET,
gettext_noop("Valid values are \"local\" and \"cascaded\".")
},
/* list terminator */
{{NULL}}
};
static relopt_string stringRelOpts[] =
{
/* list terminator */
{{NULL}}
};
static relopt_gen **relOpts = NULL;
static bits32 last_assigned_kind = RELOPT_KIND_LAST_DEFAULT;
@ -505,6 +527,12 @@ initialize_reloptions(void)
realRelOpts[i].gen.lockmode));
j++;
}
for (i = 0; enumRelOpts[i].gen.name; i++)
{
Assert(DoLockModesConflict(enumRelOpts[i].gen.lockmode,
enumRelOpts[i].gen.lockmode));
j++;
}
for (i = 0; stringRelOpts[i].gen.name; i++)
{
Assert(DoLockModesConflict(stringRelOpts[i].gen.lockmode,
@ -543,6 +571,14 @@ initialize_reloptions(void)
j++;
}
for (i = 0; enumRelOpts[i].gen.name; i++)
{
relOpts[j] = &enumRelOpts[i].gen;
relOpts[j]->type = RELOPT_TYPE_ENUM;
relOpts[j]->namelen = strlen(relOpts[j]->name);
j++;
}
for (i = 0; stringRelOpts[i].gen.name; i++)
{
relOpts[j] = &stringRelOpts[i].gen;
@ -641,6 +677,9 @@ allocate_reloption(bits32 kinds, int type, const char *name, const char *desc,
case RELOPT_TYPE_REAL:
size = sizeof(relopt_real);
break;
case RELOPT_TYPE_ENUM:
size = sizeof(relopt_enum);
break;
case RELOPT_TYPE_STRING:
size = sizeof(relopt_string);
break;
@ -661,6 +700,13 @@ allocate_reloption(bits32 kinds, int type, const char *name, const char *desc,
newoption->type = type;
newoption->lockmode = lockmode;
/*
* Set the default lock mode for this option. There is no actual way
* for a module to enforce it when declaring a custom relation option,
* so just use the highest level, which is safe for all cases.
*/
newoption->lockmode = AccessExclusiveLock;
MemoryContextSwitchTo(oldcxt);
return newoption;
@ -721,6 +767,34 @@ add_real_reloption(bits32 kinds, const char *name, const char *desc, double defa
add_reloption((relopt_gen *) newoption);
}
/*
* add_enum_reloption
* Add a new enum reloption
*
* The members array must have a terminating NULL entry.
*
* The detailmsg is shown when unsupported values are passed, and has this
* form: "Valid values are \"foo\", \"bar\", and \"bar\"."
*
* The members array and detailmsg are not copied -- caller must ensure that
* they are valid throughout the life of the process.
*/
void
add_enum_reloption(bits32 kinds, const char *name, const char *desc,
relopt_enum_elt_def *members, int default_val,
const char *detailmsg, LOCKMODE lockmode)
{
relopt_enum *newoption;
newoption = (relopt_enum *) allocate_reloption(kinds, RELOPT_TYPE_ENUM,
name, desc, lockmode);
newoption->members = members;
newoption->default_val = default_val;
newoption->detailmsg = detailmsg;
add_reloption((relopt_gen *) newoption);
}
/*
* add_string_reloption
* Add a new string reloption
@ -1237,6 +1311,37 @@ parse_one_reloption(relopt_value *option, char *text_str, int text_len,
optreal->min, optreal->max)));
}
break;
case RELOPT_TYPE_ENUM:
{
relopt_enum *optenum = (relopt_enum *) option->gen;
relopt_enum_elt_def *elt;
parsed = false;
for (elt = optenum->members; elt->string_val; elt++)
{
if (pg_strcasecmp(value, elt->string_val) == 0)
{
option->values.enum_val = elt->symbol_val;
parsed = true;
break;
}
}
if (validate && !parsed)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid value for enum option \"%s\": %s",
option->gen->name, value),
optenum->detailmsg ?
errdetail_internal("%s", _(optenum->detailmsg)) : 0));
/*
* If value is not among the allowed string values, but we are
* not asked to validate, just use the default numeric value.
*/
if (!parsed)
option->values.enum_val = optenum->default_val;
}
break;
case RELOPT_TYPE_STRING:
{
relopt_string *optstring = (relopt_string *) option->gen;
@ -1330,6 +1435,11 @@ fillRelOptions(void *rdopts, Size basesize,
options[i].values.real_val :
((relopt_real *) options[i].gen)->default_val;
break;
case RELOPT_TYPE_ENUM:
*(int *) itempos = options[i].isset ?
options[i].values.enum_val :
((relopt_enum *) options[i].gen)->default_val;
break;
case RELOPT_TYPE_STRING:
optstring = (relopt_string *) options[i].gen;
if (options[i].isset)
@ -1446,8 +1556,8 @@ view_reloptions(Datum reloptions, bool validate)
static const relopt_parse_elt tab[] = {
{"security_barrier", RELOPT_TYPE_BOOL,
offsetof(ViewOptions, security_barrier)},
{"check_option", RELOPT_TYPE_STRING,
offsetof(ViewOptions, check_option_offset)}
{"check_option", RELOPT_TYPE_ENUM,
offsetof(ViewOptions, check_option)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_VIEW, &numoptions);

25
src/backend/access/gist/gistbuild.c
View File

@ -125,15 +125,15 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo)
buildstate.indexrel = index;
buildstate.heaprel = heap;
if (index->rd_options)
{
/* Get buffering mode from the options string */
GiSTOptions *options = (GiSTOptions *) index->rd_options;
char *bufferingMode = (char *) options + options->bufferingModeOffset;
if (strcmp(bufferingMode, "on") == 0)
if (options->buffering_mode == GIST_OPTION_BUFFERING_ON)
buildstate.bufferingMode = GIST_BUFFERING_STATS;
else if (strcmp(bufferingMode, "off") == 0)
else if (options->buffering_mode == GIST_OPTION_BUFFERING_OFF)
buildstate.bufferingMode = GIST_BUFFERING_DISABLED;
else
buildstate.bufferingMode = GIST_BUFFERING_AUTO;
@ -236,25 +236,6 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo)
return result;
}
/*
* Validator for "buffering" reloption on GiST indexes. Allows "on", "off"
* and "auto" values.
*/
void
gistValidateBufferingOption(const char *value)
{
if (value == NULL ||
(strcmp(value, "on") != 0 &&
strcmp(value, "off") != 0 &&
strcmp(value, "auto") != 0))
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid value for \"buffering\" option"),
errdetail("Valid values are \"on\", \"off\", and \"auto\".")));
}
}
/*
* Attempt to switch to buffering mode.
*

2
src/backend/access/gist/gistutil.c
View File

@ -913,7 +913,7 @@ gistoptions(Datum reloptions, bool validate)
int numoptions;
static const relopt_parse_elt tab[] = {
{"fillfactor", RELOPT_TYPE_INT, offsetof(GiSTOptions, fillfactor)},
{"buffering", RELOPT_TYPE_STRING, offsetof(GiSTOptions, bufferingModeOffset)}
{"buffering", RELOPT_TYPE_ENUM, offsetof(GiSTOptions, buffering_mode)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIST,

18
src/backend/commands/view.c
View File

@ -38,24 +38,6 @@
static void checkViewTupleDesc(TupleDesc newdesc, TupleDesc olddesc);
/*---------------------------------------------------------------------
* Validator for "check_option" reloption on views. The allowed values
* are "local" and "cascaded".
*/
void
validateWithCheckOption(const char *value)
{
if (value == NULL ||
(strcmp(value, "local") != 0 &&
strcmp(value, "cascaded") != 0))
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid value for \"check_option\" option"),
errdetail("Valid values are \"local\" and \"cascaded\".")));
}
}
/*---------------------------------------------------------------------
* DefineVirtualRelation
*

10
src/include/access/gist_private.h
View File

@ -380,6 +380,14 @@ typedef struct GISTBuildBuffers
int rootlevel;
} GISTBuildBuffers;
/* GiSTOptions->buffering_mode values */
typedef enum GistOptBufferingMode
{
GIST_OPTION_BUFFERING_AUTO,
GIST_OPTION_BUFFERING_ON,
GIST_OPTION_BUFFERING_OFF
} GistOptBufferingMode;
/*
* Storage type for GiST's reloptions
*/
@ -387,7 +395,7 @@ typedef struct GiSTOptions
{
int32 vl_len_; /* varlena header (do not touch directly!) */
int fillfactor; /* page fill factor in percent (0..100) */
int bufferingModeOffset; /* use buffering build? */
GistOptBufferingMode buffering_mode; /* buffering build mode */
} GiSTOptions;
/* gist.c */

24
src/include/access/reloptions.h
View File

@ -31,6 +31,7 @@ typedef enum relopt_type
RELOPT_TYPE_BOOL,
RELOPT_TYPE_INT,
RELOPT_TYPE_REAL,
RELOPT_TYPE_ENUM,
RELOPT_TYPE_STRING
} relopt_type;
@ -80,6 +81,7 @@ typedef struct relopt_value
bool bool_val;
int int_val;
double real_val;
int enum_val;
char *string_val; /* allocated separately */
} values;
} relopt_value;
@ -107,6 +109,25 @@ typedef struct relopt_real
double max;
} relopt_real;
/*
* relopt_enum_elt_def -- One member of the array of acceptable values
* of an enum reloption.
*/
typedef struct relopt_enum_elt_def
{
const char *string_val;
int symbol_val;
} relopt_enum_elt_def;
typedef struct relopt_enum
{
relopt_gen gen;
relopt_enum_elt_def *members;
int default_val;
const char *detailmsg;
/* null-terminated array of members */
} relopt_enum;
/* validation routines for strings */
typedef void (*validate_string_relopt) (const char *value);
@ -254,6 +275,9 @@ extern void add_int_reloption(bits32 kinds, const char *name, const char *desc,
extern void add_real_reloption(bits32 kinds, const char *name, const char *desc,
double default_val, double min_val, double max_val,
LOCKMODE lockmode);
extern void add_enum_reloption(bits32 kinds, const char *name, const char *desc,
relopt_enum_elt_def *members, int default_val,
const char *detailmsg, LOCKMODE lockmode);
extern void add_string_reloption(bits32 kinds, const char *name, const char *desc,
const char *default_val, validate_string_relopt validator,
LOCKMODE lockmode);

2
src/include/commands/view.h
View File

@ -17,8 +17,6 @@
#include "catalog/objectaddress.h"
#include "nodes/parsenodes.h"
extern void validateWithCheckOption(const char *value);
extern ObjectAddress DefineView(ViewStmt *stmt, const char *queryString,
int stmt_location, int stmt_len);

25
src/include/utils/rel.h
View File

@ -327,6 +327,13 @@ typedef struct StdRdOptions
((relation)->rd_options ? \
((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))
/* ViewOptions->check_option values */
typedef enum ViewOptCheckOption
{
VIEW_OPTION_CHECK_OPTION_NOT_SET,
VIEW_OPTION_CHECK_OPTION_LOCAL,
VIEW_OPTION_CHECK_OPTION_CASCADED
} ViewOptCheckOption;
/*
* ViewOptions
@ -336,7 +343,7 @@ typedef struct ViewOptions
{
int32 vl_len_; /* varlena header (do not touch directly!) */
bool security_barrier;
int check_option_offset;
ViewOptCheckOption check_option;
} ViewOptions;
/*
@ -355,7 +362,8 @@ typedef struct ViewOptions
*/
#define RelationHasCheckOption(relation) \
((relation)->rd_options && \
((ViewOptions *) (relation)->rd_options)->check_option_offset != 0)
((ViewOptions *) (relation)->rd_options)->check_option != \
VIEW_OPTION_CHECK_OPTION_NOT_SET)
/*
* RelationHasLocalCheckOption
@ -364,10 +372,8 @@ typedef struct ViewOptions
*/
#define RelationHasLocalCheckOption(relation) \
((relation)->rd_options && \
((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
strcmp((char *) (relation)->rd_options + \
((ViewOptions *) (relation)->rd_options)->check_option_offset, \
"local") == 0 : false)
((ViewOptions *) (relation)->rd_options)->check_option == \
VIEW_OPTION_CHECK_OPTION_LOCAL)
/*
* RelationHasCascadedCheckOption
@ -376,11 +382,8 @@ typedef struct ViewOptions
*/
#define RelationHasCascadedCheckOption(relation) \
((relation)->rd_options && \
((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
strcmp((char *) (relation)->rd_options + \
((ViewOptions *) (relation)->rd_options)->check_option_offset, \
"cascaded") == 0 : false)
((ViewOptions *) (relation)->rd_options)->check_option == \
VIEW_OPTION_CHECK_OPTION_CASCADED)
/*
* RelationIsValid

37
src/test/modules/dummy_index_am/dummy_index_am.c
View File

@ -25,11 +25,16 @@ PG_MODULE_MAGIC;
void _PG_init(void);
/* parse table for fillRelOptions */
relopt_parse_elt di_relopt_tab[5];
relopt_parse_elt di_relopt_tab[6];
/* Kind of relation options for dummy index */
relopt_kind di_relopt_kind;
typedef enum DummyAmEnum {
DUMMY_AM_ENUM_ONE,
DUMMY_AM_ENUM_TWO
} DummyAmEnum;
/* Dummy index options */
typedef struct DummyIndexOptions
{
@ -37,10 +42,18 @@ typedef struct DummyIndexOptions
int option_int;
double option_real;
bool option_bool;
DummyAmEnum option_enum;
int option_string_val_offset;
int option_string_null_offset;
} DummyIndexOptions;
relopt_enum_elt_def dummyAmEnumValues[] =
{
{"one", DUMMY_AM_ENUM_ONE},
{"two", DUMMY_AM_ENUM_TWO},
{(const char *)NULL} /* list terminator */
};
/* Handler for index AM */
PG_FUNCTION_INFO_V1(dihandler);
@ -85,13 +98,23 @@ create_reloptions_table(void)
di_relopt_tab[2].opttype = RELOPT_TYPE_BOOL;
di_relopt_tab[2].offset = offsetof(DummyIndexOptions, option_bool);
add_enum_reloption(di_relopt_kind, "option_enum",
"Enum option for dummy_index_am",
dummyAmEnumValues,
DUMMY_AM_ENUM_ONE,
"Valid values are \"one\" and \"two\".",
AccessExclusiveLock);
di_relopt_tab[3].optname = "option_enum";
di_relopt_tab[3].opttype = RELOPT_TYPE_ENUM;
di_relopt_tab[3].offset = offsetof(DummyIndexOptions, option_enum);
add_string_reloption(di_relopt_kind, "option_string_val",
"String option for dummy_index_am with non-NULL default",
"DefaultValue", &validate_string_option,
AccessExclusiveLock);
di_relopt_tab[3].optname = "option_string_val";
di_relopt_tab[3].opttype = RELOPT_TYPE_STRING;
di_relopt_tab[3].offset = offsetof(DummyIndexOptions,
di_relopt_tab[4].optname = "option_string_val";
di_relopt_tab[4].opttype = RELOPT_TYPE_STRING;
di_relopt_tab[4].offset = offsetof(DummyIndexOptions,
option_string_val_offset);
/*
@ -102,9 +125,9 @@ create_reloptions_table(void)
NULL, /* description */
NULL, &validate_string_option,
AccessExclusiveLock);
di_relopt_tab[4].optname = "option_string_null";
di_relopt_tab[4].opttype = RELOPT_TYPE_STRING;
di_relopt_tab[4].offset = offsetof(DummyIndexOptions,
di_relopt_tab[5].optname = "option_string_null";
di_relopt_tab[5].opttype = RELOPT_TYPE_STRING;
di_relopt_tab[5].offset = offsetof(DummyIndexOptions,
option_string_null_offset);
}

10
src/test/modules/dummy_index_am/sql/reloptions.sql
View File

@ -32,12 +32,15 @@ ALTER INDEX dummy_test_idx SET (option_bool = true);
ALTER INDEX dummy_test_idx SET (option_real = 3.2);
ALTER INDEX dummy_test_idx SET (option_string_val = 'val2');
ALTER INDEX dummy_test_idx SET (option_string_null = NULL);
ALTER INDEX dummy_test_idx SET (option_enum = 'one');
ALTER INDEX dummy_test_idx SET (option_enum = 'three');
SELECT unnest(reloptions) FROM pg_class WHERE relname = 'dummy_test_idx';
-- ALTER INDEX .. RESET
ALTER INDEX dummy_test_idx RESET (option_int);
ALTER INDEX dummy_test_idx RESET (option_bool);
ALTER INDEX dummy_test_idx RESET (option_real);
ALTER INDEX dummy_test_idx RESET (option_enum);
ALTER INDEX dummy_test_idx RESET (option_string_val);
ALTER INDEX dummy_test_idx RESET (option_string_null);
SELECT unnest(reloptions) FROM pg_class WHERE relname = 'dummy_test_idx';
@ -62,6 +65,13 @@ ALTER INDEX dummy_test_idx SET (option_real = true); -- error
ALTER INDEX dummy_test_idx SET (option_real = 'val5'); -- error
SELECT unnest(reloptions) FROM pg_class WHERE relname = 'dummy_test_idx';
ALTER INDEX dummy_test_idx RESET (option_real);
-- Enum
ALTER INDEX dummy_test_idx SET (option_enum = 'one'); -- ok
ALTER INDEX dummy_test_idx SET (option_enum = 0); -- error
ALTER INDEX dummy_test_idx SET (option_enum = true); -- error
ALTER INDEX dummy_test_idx SET (option_enum = 'three'); -- error
SELECT unnest(reloptions) FROM pg_class WHERE relname = 'dummy_test_idx';
ALTER INDEX dummy_test_idx RESET (option_enum);
-- String
ALTER INDEX dummy_test_idx SET (option_string_val = 4); -- ok
ALTER INDEX dummy_test_idx SET (option_string_val = 3.5); -- ok

2
src/test/regress/expected/gist.out
View File

@ -12,7 +12,7 @@ create index gist_pointidx4 on gist_point_tbl using gist(p) with (buffering = au
drop index gist_pointidx2, gist_pointidx3, gist_pointidx4;
-- Make sure bad values are refused
create index gist_pointidx5 on gist_point_tbl using gist(p) with (buffering = invalid_value);
ERROR: invalid value for "buffering" option
ERROR: invalid value for enum option "buffering": invalid_value
DETAIL: Valid values are "on", "off", and "auto".
create index gist_pointidx5 on gist_point_tbl using gist(p) with (fillfactor=9);
ERROR: value 9 out of bounds for option "fillfactor"

2
src/test/regress/expected/updatable_views.out
View File

@ -1733,7 +1733,7 @@ SELECT * FROM base_tbl;
(2 rows)
ALTER VIEW rw_view1 SET (check_option=here); -- invalid
ERROR: invalid value for "check_option" option
ERROR: invalid value for enum option "check_option": here
DETAIL: Valid values are "local" and "cascaded".
ALTER VIEW rw_view1 SET (check_option=local);
INSERT INTO rw_view2 VALUES (-20); -- should fail