Skip to content

Commit 773df88

Browse files
alvherrealvherre
committed
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
1 parent caba97a commit 773df88

File tree

12 files changed

+214
-75
lines changed

12 files changed

+214
-75
lines changed

‎src/backend/access/common/reloptions.c

Lines changed: 121 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,25 @@ static relopt_real realRelOpts[] =
433433
{{NULL}}
434434
};
435435

436-
static relopt_string stringRelOpts[] =
436+
/* values from GistOptBufferingMode */
437+
relopt_enum_elt_def gistBufferingOptValues[] =
438+
{
439+
{"auto", GIST_OPTION_BUFFERING_AUTO},
440+
{"on", GIST_OPTION_BUFFERING_ON},
441+
{"off", GIST_OPTION_BUFFERING_OFF},
442+
{(const char *) NULL} /* list terminator */
443+
};
444+
445+
/* values from ViewOptCheckOption */
446+
relopt_enum_elt_def viewCheckOptValues[] =
447+
{
448+
/* no value for NOT_SET */
449+
{"local", VIEW_OPTION_CHECK_OPTION_LOCAL},
450+
{"cascaded", VIEW_OPTION_CHECK_OPTION_CASCADED},
451+
{(const char *) NULL} /* list terminator */
452+
};
453+
454+
static relopt_enum enumRelOpts[] =
437455
{
438456
{
439457
{
@@ -442,10 +460,9 @@ static relopt_string stringRelOpts[] =
442460
RELOPT_KIND_GIST,
443461
AccessExclusiveLock
444462
},
445-
4,
446-
false,
447-
gistValidateBufferingOption,
448-
"auto"
463+
gistBufferingOptValues,
464+
GIST_OPTION_BUFFERING_AUTO,
465+
gettext_noop("Valid values are \"on\", \"off\", and \"auto\".")
449466
},
450467
{
451468
{
@@ -454,15 +471,20 @@ static relopt_string stringRelOpts[] =
454471
RELOPT_KIND_VIEW,
455472
AccessExclusiveLock
456473
},
457-
0,
458-
true,
459-
validateWithCheckOption,
460-
NULL
474+
viewCheckOptValues,
475+
VIEW_OPTION_CHECK_OPTION_NOT_SET,
476+
gettext_noop("Valid values are \"local\" and \"cascaded\".")
461477
},
462478
/* list terminator */
463479
{{NULL}}
464480
};
465481

482+
static relopt_string stringRelOpts[] =
483+
{
484+
/* list terminator */
485+
{{NULL}}
486+
};
487+
466488
static relopt_gen **relOpts = NULL;
467489
static bits32 last_assigned_kind = RELOPT_KIND_LAST_DEFAULT;
468490

@@ -505,6 +527,12 @@ initialize_reloptions(void)
505527
realRelOpts[i].gen.lockmode));
506528
j++;
507529
}
530+
for (i = 0; enumRelOpts[i].gen.name; i++)
531+
{
532+
Assert(DoLockModesConflict(enumRelOpts[i].gen.lockmode,
533+
enumRelOpts[i].gen.lockmode));
534+
j++;
535+
}
508536
for (i = 0; stringRelOpts[i].gen.name; i++)
509537
{
510538
Assert(DoLockModesConflict(stringRelOpts[i].gen.lockmode,
@@ -543,6 +571,14 @@ initialize_reloptions(void)
543571
j++;
544572
}
545573

574+
for (i = 0; enumRelOpts[i].gen.name; i++)
575+
{
576+
relOpts[j] = &enumRelOpts[i].gen;
577+
relOpts[j]->type = RELOPT_TYPE_ENUM;
578+
relOpts[j]->namelen = strlen(relOpts[j]->name);
579+
j++;
580+
}
581+
546582
for (i = 0; stringRelOpts[i].gen.name; i++)
547583
{
548584
relOpts[j] = &stringRelOpts[i].gen;
@@ -641,6 +677,9 @@ allocate_reloption(bits32 kinds, int type, const char *name, const char *desc,
641677
case RELOPT_TYPE_REAL:
642678
size = sizeof(relopt_real);
643679
break;
680+
case RELOPT_TYPE_ENUM:
681+
size = sizeof(relopt_enum);
682+
break;
644683
case RELOPT_TYPE_STRING:
645684
size = sizeof(relopt_string);
646685
break;
@@ -661,6 +700,13 @@ allocate_reloption(bits32 kinds, int type, const char *name, const char *desc,
661700
newoption->type = type;
662701
newoption->lockmode = lockmode;
663702

703+
/*
704+
* Set the default lock mode for this option. There is no actual way
705+
* for a module to enforce it when declaring a custom relation option,
706+
* so just use the highest level, which is safe for all cases.
707+
*/
708+
newoption->lockmode = AccessExclusiveLock;
709+
664710
MemoryContextSwitchTo(oldcxt);
665711

666712
return newoption;
@@ -721,6 +767,34 @@ add_real_reloption(bits32 kinds, const char *name, const char *desc, double defa
721767
add_reloption((relopt_gen *) newoption);
722768
}
723769

770+
/*
771+
* add_enum_reloption
772+
* Add a new enum reloption
773+
*
774+
* The members array must have a terminating NULL entry.
775+
*
776+
* The detailmsg is shown when unsupported values are passed, and has this
777+
* form: "Valid values are \"foo\", \"bar\", and \"bar\"."
778+
*
779+
* The members array and detailmsg are not copied -- caller must ensure that
780+
* they are valid throughout the life of the process.
781+
*/
782+
void
783+
add_enum_reloption(bits32 kinds, const char *name, const char *desc,
784+
relopt_enum_elt_def *members, int default_val,
785+
const char *detailmsg, LOCKMODE lockmode)
786+
{
787+
relopt_enum *newoption;
788+
789+
newoption = (relopt_enum *) allocate_reloption(kinds, RELOPT_TYPE_ENUM,
790+
name, desc, lockmode);
791+
newoption->members = members;
792+
newoption->default_val = default_val;
793+
newoption->detailmsg = detailmsg;
794+
795+
add_reloption((relopt_gen *) newoption);
796+
}
797+
724798
/*
725799
* add_string_reloption
726800
* Add a new string reloption
@@ -1237,6 +1311,37 @@ parse_one_reloption(relopt_value *option, char *text_str, int text_len,
12371311
optreal->min, optreal->max)));
12381312
}
12391313
break;
1314+
case RELOPT_TYPE_ENUM:
1315+
{
1316+
relopt_enum *optenum = (relopt_enum *) option->gen;
1317+
relopt_enum_elt_def *elt;
1318+
1319+
parsed = false;
1320+
for (elt = optenum->members; elt->string_val; elt++)
1321+
{
1322+
if (pg_strcasecmp(value, elt->string_val) == 0)
1323+
{
1324+
option->values.enum_val = elt->symbol_val;
1325+
parsed = true;
1326+
break;
1327+
}
1328+
}
1329+
if (validate && !parsed)
1330+
ereport(ERROR,
1331+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
1332+
errmsg("invalid value for enum option \"%s\": %s",
1333+
option->gen->name, value),
1334+
optenum->detailmsg ?
1335+
errdetail_internal("%s", _(optenum->detailmsg)) : 0));
1336+
1337+
/*
1338+
* If value is not among the allowed string values, but we are
1339+
* not asked to validate, just use the default numeric value.
1340+
*/
1341+
if (!parsed)
1342+
option->values.enum_val = optenum->default_val;
1343+
}
1344+
break;
12401345
case RELOPT_TYPE_STRING:
12411346
{
12421347
relopt_string *optstring = (relopt_string *) option->gen;
@@ -1330,6 +1435,11 @@ fillRelOptions(void *rdopts, Size basesize,
13301435
options[i].values.real_val :
13311436
((relopt_real *) options[i].gen)->default_val;
13321437
break;
1438+
case RELOPT_TYPE_ENUM:
1439+
*(int *) itempos = options[i].isset ?
1440+
options[i].values.enum_val :
1441+
((relopt_enum *) options[i].gen)->default_val;
1442+
break;
13331443
case RELOPT_TYPE_STRING:
13341444
optstring = (relopt_string *) options[i].gen;
13351445
if (options[i].isset)
@@ -1446,8 +1556,8 @@ view_reloptions(Datum reloptions, bool validate)
14461556
static const relopt_parse_elt tab[] = {
14471557
{"security_barrier", RELOPT_TYPE_BOOL,
14481558
offsetof(ViewOptions, security_barrier)},
1449-
{"check_option", RELOPT_TYPE_STRING,
1450-
offsetof(ViewOptions, check_option_offset)}
1559+
{"check_option", RELOPT_TYPE_ENUM,
1560+
offsetof(ViewOptions, check_option)}
14511561
};
14521562

14531563
options = parseRelOptions(reloptions, validate, RELOPT_KIND_VIEW, &numoptions);

‎src/backend/access/gist/gistbuild.c

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -125,15 +125,15 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo)
125125

126126
buildstate.indexrel = index;
127127
buildstate.heaprel = heap;
128+
128129
if (index->rd_options)
129130
{
130131
/* Get buffering mode from the options string */
131132
GiSTOptions *options = (GiSTOptions *) index->rd_options;
132-
char *bufferingMode = (char *) options + options->bufferingModeOffset;
133133

134-
if (strcmp(bufferingMode, "on") == 0)
134+
if (options->buffering_mode == GIST_OPTION_BUFFERING_ON)
135135
buildstate.bufferingMode = GIST_BUFFERING_STATS;
136-
else if (strcmp(bufferingMode, "off") == 0)
136+
else if (options->buffering_mode == GIST_OPTION_BUFFERING_OFF)
137137
buildstate.bufferingMode = GIST_BUFFERING_DISABLED;
138138
else
139139
buildstate.bufferingMode = GIST_BUFFERING_AUTO;
@@ -236,25 +236,6 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo)
236236
return result;
237237
}
238238

239-
/*
240-
* Validator for "buffering" reloption on GiST indexes. Allows "on", "off"
241-
* and "auto" values.
242-
*/
243-
void
244-
gistValidateBufferingOption(const char *value)
245-
{
246-
if (value == NULL ||
247-
(strcmp(value, "on") != 0 &&
248-
strcmp(value, "off") != 0 &&
249-
strcmp(value, "auto") != 0))
250-
{
251-
ereport(ERROR,
252-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
253-
errmsg("invalid value for \"buffering\" option"),
254-
errdetail("Valid values are \"on\", \"off\", and \"auto\".")));
255-
}
256-
}
257-
258239
/*
259240
* Attempt to switch to buffering mode.
260241
*

‎src/backend/access/gist/gistutil.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,7 @@ gistoptions(Datum reloptions, bool validate)
913913
int numoptions;
914914
static const relopt_parse_elt tab[] = {
915915
{"fillfactor", RELOPT_TYPE_INT, offsetof(GiSTOptions, fillfactor)},
916-
{"buffering", RELOPT_TYPE_STRING, offsetof(GiSTOptions, bufferingModeOffset)}
916+
{"buffering", RELOPT_TYPE_ENUM, offsetof(GiSTOptions, buffering_mode)}
917917
};
918918

919919
options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIST,

‎src/backend/commands/view.c

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -38,24 +38,6 @@
3838

3939
static void checkViewTupleDesc(TupleDesc newdesc, TupleDesc olddesc);
4040

41-
/*---------------------------------------------------------------------
42-
* Validator for "check_option" reloption on views. The allowed values
43-
* are "local" and "cascaded".
44-
*/
45-
void
46-
validateWithCheckOption(const char *value)
47-
{
48-
if (value == NULL ||
49-
(strcmp(value, "local") != 0 &&
50-
strcmp(value, "cascaded") != 0))
51-
{
52-
ereport(ERROR,
53-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
54-
errmsg("invalid value for \"check_option\" option"),
55-
errdetail("Valid values are \"local\" and \"cascaded\".")));
56-
}
57-
}
58-
5941
/*---------------------------------------------------------------------
6042
* DefineVirtualRelation
6143
*

‎src/include/access/gist_private.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,14 +380,22 @@ typedef struct GISTBuildBuffers
380380
int rootlevel;
381381
} GISTBuildBuffers;
382382

383+
/* GiSTOptions->buffering_mode values */
384+
typedef enum GistOptBufferingMode
385+
{
386+
GIST_OPTION_BUFFERING_AUTO,
387+
GIST_OPTION_BUFFERING_ON,
388+
GIST_OPTION_BUFFERING_OFF
389+
} GistOptBufferingMode;
390+
383391
/*
384392
* Storage type for GiST's reloptions
385393
*/
386394
typedef struct GiSTOptions
387395
{
388396
int32 vl_len_; /* varlena header (do not touch directly!) */
389397
int fillfactor; /* page fill factor in percent (0..100) */
390-
int bufferingModeOffset; /* use buffering build? */
398+
GistOptBufferingMode buffering_mode; /* buffering build mode */
391399
} GiSTOptions;
392400

393401
/* gist.c */

‎src/include/access/reloptions.h

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ typedef enum relopt_type
3131
RELOPT_TYPE_BOOL,
3232
RELOPT_TYPE_INT,
3333
RELOPT_TYPE_REAL,
34+
RELOPT_TYPE_ENUM,
3435
RELOPT_TYPE_STRING
3536
} relopt_type;
3637

@@ -80,6 +81,7 @@ typedef struct relopt_value
8081
bool bool_val;
8182
int int_val;
8283
double real_val;
84+
int enum_val;
8385
char *string_val; /* allocated separately */
8486
} values;
8587
} relopt_value;
@@ -107,6 +109,25 @@ typedef struct relopt_real
107109
double max;
108110
} relopt_real;
109111

112+
/*
113+
* relopt_enum_elt_def -- One member of the array of acceptable values
114+
* of an enum reloption.
115+
*/
116+
typedef struct relopt_enum_elt_def
117+
{
118+
const char *string_val;
119+
int symbol_val;
120+
} relopt_enum_elt_def;
121+
122+
typedef struct relopt_enum
123+
{
124+
relopt_gen gen;
125+
relopt_enum_elt_def *members;
126+
int default_val;
127+
const char *detailmsg;
128+
/* null-terminated array of members */
129+
} relopt_enum;
130+
110131
/* validation routines for strings */
111132
typedef void (*validate_string_relopt) (const char *value);
112133

@@ -254,6 +275,9 @@ extern void add_int_reloption(bits32 kinds, const char *name, const char *desc,
254275
extern void add_real_reloption(bits32 kinds, const char *name, const char *desc,
255276
double default_val, double min_val, double max_val,
256277
LOCKMODE lockmode);
278+
extern void add_enum_reloption(bits32 kinds, const char *name, const char *desc,
279+
relopt_enum_elt_def *members, int default_val,
280+
const char *detailmsg, LOCKMODE lockmode);
257281
extern void add_string_reloption(bits32 kinds, const char *name, const char *desc,
258282
const char *default_val, validate_string_relopt validator,
259283
LOCKMODE lockmode);

‎src/include/commands/view.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
#include "catalog/objectaddress.h"
1818
#include "nodes/parsenodes.h"
1919

20-
extern void validateWithCheckOption(const char *value);
21-
2220
extern ObjectAddress DefineView(ViewStmt *stmt, const char *queryString,
2321
int stmt_location, int stmt_len);
2422

0 commit comments

Comments
 (0)