From d9034c94d496ba497f65acfe3c80b57b08b4a95c Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Mon, 24 Mar 2025 12:46:26 -0500 Subject: [PATCH v2 1/1] change vacuum_truncate relopt to enum --- src/backend/access/common/reloptions.c | 89 +++++++++++++++++++------- src/backend/commands/vacuum.c | 4 +- src/include/access/reloptions.h | 13 ---- src/include/utils/rel.h | 15 ++++- src/tools/pgindent/typedefs.list | 1 + 5 files changed, 82 insertions(+), 40 deletions(-) diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c index 645b5c00467..a011a932e7d 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -147,15 +147,6 @@ static relopt_bool boolRelOpts[] = }, false }, - { - { - "vacuum_truncate", - "Enables vacuum to truncate empty pages at the end of this table", - RELOPT_KIND_HEAP | RELOPT_KIND_TOAST, - ShareUpdateExclusiveLock - }, - true - }, { { "deduplicate_items", @@ -524,6 +515,43 @@ static relopt_enum_elt_def viewCheckOptValues[] = {(const char *) NULL} /* list terminator */ }; +/* + * The values are from StdRdOptBool. + * + * This enum is meant to be used for Boolean reloptions for which we need to + * be able to determine whether the value was explicitly set for the relation. + * There is a third unusable StdRdOptBool value called + * STDRD_OPTION_BOOL_NOT_SET that should be set as the default value for such + * options. Then, code that uses the option can first test for the "not set" + * value and fall back to something else (e.g., a GUC) as needed. + * + * Note that the strings below have been carefully chosen to match the behavior + * of parse_bool(). + */ +static relopt_enum_elt_def StdRdOptBoolValues[] = +{ + {"on", STDRD_OPTION_BOOL_ON}, + {"of", STDRD_OPTION_BOOL_OFF}, + {"off", STDRD_OPTION_BOOL_OFF}, + {"t", STDRD_OPTION_BOOL_ON}, + {"tr", STDRD_OPTION_BOOL_ON}, + {"tru", STDRD_OPTION_BOOL_ON}, + {"true", STDRD_OPTION_BOOL_ON}, + {"f", STDRD_OPTION_BOOL_OFF}, + {"fa", STDRD_OPTION_BOOL_OFF}, + {"fal", STDRD_OPTION_BOOL_OFF}, + {"fals", STDRD_OPTION_BOOL_OFF}, + {"false", STDRD_OPTION_BOOL_OFF}, + {"y", STDRD_OPTION_BOOL_ON}, + {"ye", STDRD_OPTION_BOOL_ON}, + {"yes", STDRD_OPTION_BOOL_ON}, + {"n", STDRD_OPTION_BOOL_OFF}, + {"no", STDRD_OPTION_BOOL_OFF}, + {"1", STDRD_OPTION_BOOL_ON}, + {"0", STDRD_OPTION_BOOL_OFF}, + {(const char *) NULL} /* list terminator */ +}; + static relopt_enum enumRelOpts[] = { { @@ -559,6 +587,16 @@ static relopt_enum enumRelOpts[] = VIEW_OPTION_CHECK_OPTION_NOT_SET, gettext_noop("Valid values are \"local\" and \"cascaded\".") }, + { + { + "vacuum_truncate", + "Enables vacuum to truncate empty pages at the end of this table", + RELOPT_KIND_HEAP | RELOPT_KIND_TOAST, + ShareUpdateExclusiveLock + }, + StdRdOptBoolValues, + STDRD_OPTION_BOOL_NOT_SET + }, /* list terminator */ {{NULL}} }; @@ -1672,12 +1710,29 @@ parse_one_reloption(relopt_value *option, char *text_str, int text_len, } } if (validate && !parsed) + { + /* + * If the enum is using StdRdOptBoolValues, it is actually + * a Boolean reloption for all intents and purposes, but + * we need to be able to use a third value of "not set" as + * the default for the reloption to be able to determine + * whether it is actually set for the relation. To uphold + * the illusion that this is a Boolean reloption, we emit + * the same error message as for RELOPT_TYPE_BOOL above. + */ + if (optenum->members == StdRdOptBoolValues) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid value for boolean option \"%s\": %s", + option->gen->name, value))); + 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 @@ -1779,17 +1834,6 @@ fillRelOptions(void *rdopts, Size basesize, char *itempos = ((char *) rdopts) + elems[j].offset; char *string_val; - /* - * If isset_offset is provided, store whether the reloption is - * set there. - */ - if (elems[j].isset_offset > 0) - { - char *setpos = ((char *) rdopts) + elems[j].isset_offset; - - *(bool *) setpos = options[i].isset; - } - switch (options[i].gen->type) { case RELOPT_TYPE_BOOL: @@ -1911,8 +1955,8 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind) offsetof(StdRdOptions, parallel_workers)}, {"vacuum_index_cleanup", RELOPT_TYPE_ENUM, offsetof(StdRdOptions, vacuum_index_cleanup)}, - {"vacuum_truncate", RELOPT_TYPE_BOOL, - offsetof(StdRdOptions, vacuum_truncate), offsetof(StdRdOptions, vacuum_truncate_set)}, + {"vacuum_truncate", RELOPT_TYPE_ENUM, + offsetof(StdRdOptions, vacuum_truncate)}, {"vacuum_max_eager_freeze_failure_rate", RELOPT_TYPE_REAL, offsetof(StdRdOptions, vacuum_max_eager_freeze_failure_rate)} }; @@ -1992,7 +2036,6 @@ build_local_reloptions(local_relopts *relopts, Datum options, bool validate) elems[i].optname = opt->option->name; elems[i].opttype = opt->option->type; elems[i].offset = opt->offset; - elems[i].isset_offset = 0; /* not supported for local relopts yet */ i++; } diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index f0a7b87808d..7c725ea4b76 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -2206,9 +2206,9 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, { StdRdOptions *opts = (StdRdOptions *) rel->rd_options; - if (opts && opts->vacuum_truncate_set) + if (opts && opts->vacuum_truncate != STDRD_OPTION_BOOL_NOT_SET) { - if (opts->vacuum_truncate) + if (opts->vacuum_truncate == STDRD_OPTION_BOOL_ON) params->truncate = VACOPTVALUE_ENABLED; else params->truncate = VACOPTVALUE_DISABLED; diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h index dfbb4c85460..43445cdcc6c 100644 --- a/src/include/access/reloptions.h +++ b/src/include/access/reloptions.h @@ -152,19 +152,6 @@ typedef struct const char *optname; /* option's name */ relopt_type opttype; /* option's datatype */ int offset; /* offset of field in result struct */ - - /* - * isset_offset is an optional offset of a field in the result struct that - * stores whether the option is explicitly set for the relation or if it - * just picked up the default value. In most cases, this can be - * accomplished by giving the reloption a special out-of-range default - * value (e.g., some integer reloptions use -2), but this isn't always - * possible. For example, a Boolean reloption cannot be given an - * out-of-range default, so we need another way to discover the source of - * its value. This offset is only used if given a value greater than - * zero. - */ - int isset_offset; } relopt_parse_elt; /* Local reloption definition */ diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index d94fddd7cef..fbcc12a9cc7 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -334,6 +334,18 @@ typedef enum StdRdOptIndexCleanup STDRD_OPTION_VACUUM_INDEX_CLEANUP_ON, } StdRdOptIndexCleanup; +/* + * This is useful for Boolean relopts when we need to be able to determine + * whether it's explicitly set for a relation. See comment atop + * StdRdOptBoolValues in reloptions.c for details. + */ +typedef enum StdRdOptBool +{ + STDRD_OPTION_BOOL_NOT_SET = 0, + STDRD_OPTION_BOOL_ON, + STDRD_OPTION_BOOL_OFF, +} StdRdOptBool; + typedef struct StdRdOptions { int32 vl_len_; /* varlena header (do not touch directly!) */ @@ -343,8 +355,7 @@ typedef struct StdRdOptions bool user_catalog_table; /* use as an additional catalog relation */ int parallel_workers; /* max number of parallel workers */ StdRdOptIndexCleanup vacuum_index_cleanup; /* controls index vacuuming */ - bool vacuum_truncate; /* enables vacuum to truncate a relation */ - bool vacuum_truncate_set; /* whether vacuum_truncate is set */ + StdRdOptBool vacuum_truncate; /* enables vacuum to truncate a relation */ /* * Fraction of pages in a relation that vacuum can eagerly scan and fail diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 3fbf5a4c212..ac530fb40e7 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2818,6 +2818,7 @@ StatsData StatsElem StatsExtInfo StdAnalyzeData +StdRdOptBool StdRdOptIndexCleanup StdRdOptions Step -- 2.39.5 (Apple Git-154)