On Thu, Mar 20, 2025 at 2:31 PM Nathan Bossart <nathandboss...@gmail.com>
wrote:
> On Thu, Mar 20, 2025 at 02:18:33PM -0700, David G. Johnston wrote:
> > So my concern about dump/restore seems to be alleviated but then, why can
> > we not just do whatever pg_dump is doing to decide whether the current
> > value for vacuum_truncate is its default (and thus would not be dumped)
> or
> > not (and would be dumped)?
>
> pg_dump looks at the pg_class.reloptions array directly. In the vacuum
> code, we look at the pre-parsed rd_options (see RelationParseRelOptions()
> in relcache.c), which will have already resolved vacuum_truncate to its
> default value if it was not explicitly set. We could probably look at
> pg_class.reloptions directly in the vacuum code if we _really_ wanted to,
> but I felt that putting this information into rd_options was much cleaner.
>
>
I understand this now and suggest the following comment changes based upon
that understanding.
diff --git a/src/backend/access/common/reloptions.c
b/src/backend/access/common/reloptions.c
index 645b5c0046..c795df6100 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -1912,7 +1912,9 @@ default_reloptions(Datum reloptions, bool validate,
relopt_kind kind)
{"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)},
+ offsetof(StdRdOptions, vacuum_truncate),
+ /* vacuum_truncate uses the isset_offset member instead of
a sentinel value */
+ offsetof(StdRdOptions, vacuum_truncate_set)},
{"vacuum_max_eager_freeze_failure_rate", RELOPT_TYPE_REAL,
offsetof(StdRdOptions,
vacuum_max_eager_freeze_failure_rate)}
};
diff --git a/src/include/access/reloptions.h
b/src/include/access/reloptions.h
index 146aed47c2..fddeee0d54 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -152,7 +152,17 @@ typedef struct
const char *optname; /* option's name */
relopt_type opttype; /* option's datatype */
int offset; /* offset of field
in result struct */
- int isset_offset; /* if > 0, offset of "is
set" field */
+ /*
+ * The reloptions system knows whether a reloption has been set by
the
+ * DBA or not. Historically, this information was lost when parsing
+ * the reloptions so we coped by using sentinel values.
+ * This doesn't work for boolean reloptions. For those, we
+ * need a place for the reloption parser to store its knowledge of
+ * whether the reloption was set. Set this field to an offset
+ * in the StdRdOptions struct. The pointed-to location needs to
+ * be a bool. A value of 0 here is interpreted as "no need to
store".
+ */
+ int isset_offset;
} relopt_parse_elt;
/* Local reloption definition */
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index d94fddd7ce..461648aac6 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -344,7 +344,7 @@ typedef struct StdRdOptions
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 */
+ bool vacuum_truncate_set; /* reserve space for isset
status of vacuum_truncate */
/*
* Fraction of pages in a relation that vacuum can eagerly scan and
fail
David J.
