On Wed, Nov 13, 2019 at 10:52:52AM +0900, Amit Langote wrote: > Thanks for chiming in about that. I guess that means that we don't > need those macros, except GET_STRING_RELOPTION_LEN() that's used in > allocateReloptStruct(), which can be moved to reloptions.c. Is that > correct?
I have been looking on the net to see if there are any traces of code using those macros, but could not find any. The last trace of a macro use is in 8ebe1e3, which just relies on GET_STRING_RELOPTION_LEN. So it looks rather convincing now to just remove this code. Attached is a patch for that. There could be an argument for keeping GET_STRING_RELOPTION actually which is still useful to get a string value in an option set using the stored offset, and we have the recently-added dummy_index_am in this category. Any thoughts? -- Michael
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index db42aa35e0..e24fdd1975 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -148,124 +148,6 @@ typedef struct
int offset; /* offset of field in result struct */
} relopt_parse_elt;
-
-/*
- * These macros exist for the convenience of amoptions writers (but consider
- * using fillRelOptions, which is a lot simpler). Beware of multiple
- * evaluation of arguments!
- *
- * The last argument in the HANDLE_*_RELOPTION macros allows the caller to
- * determine whether the option was set (true), or its value acquired from
- * defaults (false); it can be passed as (char *) NULL if the caller does not
- * need this information.
- *
- * optname is the option name (a string), var is the variable
- * on which the value should be stored (e.g. StdRdOptions->fillfactor), and
- * option is a relopt_value pointer.
- *
- * The normal way to use this is to loop on the relopt_value array returned by
- * parseRelOptions:
- * for (i = 0; options[i].gen->name; i++)
- * {
- * if (HAVE_RELOPTION("fillfactor", options[i])
- * {
- * HANDLE_INT_RELOPTION("fillfactor", rdopts->fillfactor, options[i], &isset);
- * continue;
- * }
- * if (HAVE_RELOPTION("default_row_acl", options[i])
- * {
- * ...
- * }
- * ...
- * if (validate)
- * ereport(ERROR,
- * (errmsg("unknown option")));
- * }
- *
- * Note that this is more or less the same that fillRelOptions does, so only
- * use this if you need to do something non-standard within some option's
- * code block.
- */
-#define HAVE_RELOPTION(optname, option) \
- (strncmp(option.gen->name, optname, option.gen->namelen + 1) == 0)
-
-#define HANDLE_INT_RELOPTION(optname, var, option, wasset) \
- do { \
- if (option.isset) \
- var = option.values.int_val; \
- else \
- var = ((relopt_int *) option.gen)->default_val; \
- (wasset) != NULL ? *(wasset) = option.isset : (dummyret)NULL; \
- } while (0)
-
-#define HANDLE_BOOL_RELOPTION(optname, var, option, wasset) \
- do { \
- if (option.isset) \
- var = option.values.bool_val; \
- else \
- var = ((relopt_bool *) option.gen)->default_val; \
- (wasset) != NULL ? *(wasset) = option.isset : (dummyret) NULL; \
- } while (0)
-
-#define HANDLE_REAL_RELOPTION(optname, var, option, wasset) \
- do { \
- if (option.isset) \
- var = option.values.real_val; \
- else \
- var = ((relopt_real *) option.gen)->default_val; \
- (wasset) != NULL ? *(wasset) = option.isset : (dummyret) NULL; \
- } while (0)
-
-/*
- * Note that this assumes that the variable is already allocated at the tail of
- * reloptions structure (StdRdOptions or equivalent).
- *
- * "base" is a pointer to the reloptions structure, and "offset" is an integer
- * variable that must be initialized to sizeof(reloptions structure). This
- * struct must have been allocated with enough space to hold any string option
- * present, including terminating \0 for every option. SET_VARSIZE() must be
- * called on the struct with this offset as the second argument, after all the
- * string options have been processed.
- */
-#define HANDLE_STRING_RELOPTION(optname, var, option, base, offset, wasset) \
- do { \
- relopt_string *optstring = (relopt_string *) option.gen;\
- char *string_val; \
- if (option.isset) \
- string_val = option.values.string_val; \
- else if (!optstring->default_isnull) \
- string_val = optstring->default_val; \
- else \
- string_val = NULL; \
- (wasset) != NULL ? *(wasset) = option.isset : (dummyret) NULL; \
- if (string_val == NULL) \
- var = 0; \
- else \
- { \
- strcpy(((char *)(base)) + (offset), string_val); \
- var = (offset); \
- (offset) += strlen(string_val) + 1; \
- } \
- } while (0)
-
-/*
- * For use during amoptions: get the strlen of a string option
- * (either default or the user defined value)
- */
-#define GET_STRING_RELOPTION_LEN(option) \
- ((option).isset ? strlen((option).values.string_val) : \
- ((relopt_string *) (option).gen)->default_len)
-
-/*
- * For use by code reading options already parsed: get a pointer to the string
- * value itself. "optstruct" is the StdRdOptions struct or equivalent, "member"
- * is the struct member corresponding to the string option
- */
-#define GET_STRING_RELOPTION(optstruct, member) \
- ((optstruct)->member == 0 ? NULL : \
- (char *)(optstruct) + (optstruct)->member)
-
-
extern relopt_kind add_reloption_kind(void);
extern void add_bool_reloption(bits32 kinds, const char *name, const char *desc,
bool default_val, LOCKMODE lockmode);
@@ -288,14 +170,6 @@ extern Datum transformRelOptions(Datum oldOptions, List *defList,
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);
-extern void *allocateReloptStruct(Size base, relopt_value *options,
- int numoptions);
-extern void fillRelOptions(void *rdopts, Size basesize,
- relopt_value *options, int numoptions,
- bool validate,
- const relopt_parse_elt *elems, int nelems);
extern void *build_reloptions(Datum reloptions, bool validate,
relopt_kind kind,
Size relopt_struct_size,
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index d8790ad7a3..8240ff1fd6 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -496,6 +496,15 @@ static void initialize_reloptions(void);
static void parse_one_reloption(relopt_value *option, char *text_str,
int text_len, bool validate);
+/*
+ * Get the length of a string reloption (either default or the user-defined
+ * value). This is used for allocation purposes when building a set of
+ * relation options.
+ */
+#define GET_STRING_RELOPTION_LEN(option) \
+ ((option).isset ? strlen((option).values.string_val) : \
+ ((relopt_string *) (option).gen)->default_len)
+
/*
* initialize_reloptions
* initialization routine, must be called before parsing
@@ -1140,7 +1149,7 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
* returned array. Values of type string are allocated separately and must
* be freed by the caller.
*/
-relopt_value *
+static relopt_value *
parseRelOptions(Datum options, bool validate, relopt_kind kind,
int *numrelopts)
{
@@ -1365,7 +1374,7 @@ parse_one_reloption(relopt_value *option, char *text_str, int text_len,
* "base" should be sizeof(struct) of the reloptions struct (StdRdOptions or
* equivalent).
*/
-void *
+static void *
allocateReloptStruct(Size base, relopt_value *options, int numoptions)
{
Size size = base;
@@ -1389,7 +1398,7 @@ allocateReloptStruct(Size base, relopt_value *options, int numoptions)
* elems, of length numelems, is the table describing the allowed options.
* When validate is true, it is expected that all options appear in elems.
*/
-void
+static void
fillRelOptions(void *rdopts, Size basesize,
relopt_value *options, int numoptions,
bool validate,
signature.asc
Description: PGP signature
