I took a quick look at 0001+0002 and I think it's quite reasonable. Here it is again with some minor fixups. (I'm omitting the further patches for now, we can rebase them later.)
I'm CCing Nathan as committer of the vacuum_truncate_set stuff which Nikolay so strongly disliked. Any objections to going with this approach? Thanks, (Please note that Gmail is sabotaging my kurilemu.de domain, so there's significant delay in my emails to the list from that address. I guess I'm lucky that Nikolay decided to CC my old address in this thread.) -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "The problem with the future is that it keeps turning into the present" (Hobbes)
>From fe54ef4634f25bced3d8651da28f7728b53b019c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <[email protected]> Date: Fri, 16 Jan 2026 16:04:22 +0100 Subject: [PATCH v3] Introduce ternary reloptions Introduce ternary reloption as a replacement for current `vacuum_truncate` implementation. Remove the `vacuum_truncate_set` separate flag and use TERNARY_UNSET instead. This could also be used for other options such as `vacuum_index_cleanup` and `buffering`, but lets get the scaffolding in first. Discussion: https://postgr.es/m/3474141.usfYGdeWWP@thinkpad-pgpro --- src/backend/access/common/reloptions.c | 137 ++++++++++++++++++----- src/backend/commands/vacuum.c | 4 +- src/include/access/reloptions.h | 27 ++--- src/include/postgres.h | 15 +++ src/include/utils/rel.h | 3 +- src/test/regress/expected/reloptions.out | 36 ++++++ src/test/regress/sql/reloptions.sql | 21 ++++ src/tools/pgindent/typedefs.list | 2 + 8 files changed, 202 insertions(+), 43 deletions(-) diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c index 0b83f98ed5f..6f1f577581a 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -40,9 +40,9 @@ * * To add an option: * - * (i) decide on a type (bool, integer, real, enum, string), name, default - * value, upper and lower bounds (if applicable); for strings, consider a - * validation routine. + * (i) decide on a type (bool, ternary, integer, real, enum, string), name, + * default value, upper and lower bounds (if applicable); for strings, + * consider a validation routine. * (ii) add a record below (or use add_<type>_reloption). * (iii) add it to the appropriate options struct (perhaps StdRdOptions) * (iv) add it to the appropriate handling routine (perhaps @@ -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", @@ -170,6 +161,25 @@ static relopt_bool boolRelOpts[] = {{NULL}} }; +static relopt_ternary ternaryRelOpts[] = +{ + { + { + "vacuum_truncate", + "Enables vacuum to truncate empty pages at the end of this table", + RELOPT_KIND_HEAP | RELOPT_KIND_TOAST, + ShareUpdateExclusiveLock + }, + TERNARY_UNSET + }, + /* list terminator */ + { + { + NULL + } + } +}; + static relopt_int intRelOpts[] = { { @@ -609,6 +619,13 @@ initialize_reloptions(void) boolRelOpts[i].gen.lockmode)); j++; } + for (i = 0; ternaryRelOpts[i].gen.name; i++) + { + Assert(DoLockModesConflict(ternaryRelOpts[i].gen.lockmode, + ternaryRelOpts[i].gen.lockmode)); + j++; + } + for (i = 0; intRelOpts[i].gen.name; i++) { Assert(DoLockModesConflict(intRelOpts[i].gen.lockmode, @@ -649,6 +666,14 @@ initialize_reloptions(void) j++; } + for (i = 0; ternaryRelOpts[i].gen.name; i++) + { + relOpts[j] = &ternaryRelOpts[i].gen; + relOpts[j]->type = RELOPT_TYPE_TERNARY; + relOpts[j]->namelen = strlen(relOpts[j]->name); + j++; + } + for (i = 0; intRelOpts[i].gen.name; i++) { relOpts[j] = &intRelOpts[i].gen; @@ -809,6 +834,9 @@ allocate_reloption(bits32 kinds, int type, const char *name, const char *desc, case RELOPT_TYPE_BOOL: size = sizeof(relopt_bool); break; + case RELOPT_TYPE_TERNARY: + size = sizeof(relopt_ternary); + break; case RELOPT_TYPE_INT: size = sizeof(relopt_int); break; @@ -892,6 +920,57 @@ add_local_bool_reloption(local_relopts *relopts, const char *name, add_local_reloption(relopts, (relopt_gen *) newoption, offset); } +/* + * init_ternary_reloption + * Allocate and initialize a new ternary reloption + */ +static relopt_ternary * +init_ternary_reloption(bits32 kinds, const char *name, const char *desc, + pg_ternary default_val, LOCKMODE lockmode) +{ + relopt_ternary *newoption; + + newoption = (relopt_ternary *) + allocate_reloption(kinds, RELOPT_TYPE_TERNARY, name, desc, lockmode); + newoption->default_val = default_val; + + return newoption; +} + +/* + * add_ternary_reloption + * Add a new ternary reloption + */ +void +add_ternary_reloption(bits32 kinds, const char *name, const char *desc, + pg_ternary default_val, LOCKMODE lockmode) +{ + relopt_ternary *newoption; + + newoption = + init_ternary_reloption(kinds, name, desc, default_val, lockmode); + + add_reloption((relopt_gen *) newoption); +} + +/* + * add_local_ternary_reloption + * Add a new ternary local reloption + * + * 'offset' is offset of ternary-typed field. + */ +void +add_local_ternary_reloption(local_relopts *relopts, const char *name, + const char *desc, pg_ternary default_val, + int offset) +{ + relopt_ternary *newoption; + + newoption = + init_ternary_reloption(RELOPT_KIND_LOCAL, name, desc, default_val, 0); + + add_local_reloption(relopts, (relopt_gen *) newoption, offset); +} /* * init_real_reloption @@ -1626,6 +1705,19 @@ parse_one_reloption(relopt_value *option, char *text_str, int text_len, option->gen->name, value))); } break; + case RELOPT_TYPE_TERNARY: + { + bool b; + + parsed = parse_bool(value, &b); + option->values.ternary_val = b ? TERNARY_TRUE : TERNARY_FALSE; + if (validate && !parsed) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid value for ternary option \"%s\": %s", + option->gen->name, value))); + } + break; case RELOPT_TYPE_INT: { relopt_int *optint = (relopt_int *) option->gen; @@ -1789,17 +1881,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: @@ -1807,6 +1888,11 @@ fillRelOptions(void *rdopts, Size basesize, options[i].values.bool_val : ((relopt_bool *) options[i].gen)->default_val; break; + case RELOPT_TYPE_TERNARY: + *(pg_ternary *) itempos = options[i].isset ? + options[i].values.ternary_val : + ((relopt_ternary *) options[i].gen)->default_val; + break; case RELOPT_TYPE_INT: *(int *) itempos = options[i].isset ? options[i].values.int_val : @@ -1923,8 +2009,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_TERNARY, + offsetof(StdRdOptions, vacuum_truncate)}, {"vacuum_max_eager_freeze_failure_rate", RELOPT_TYPE_REAL, offsetof(StdRdOptions, vacuum_max_eager_freeze_failure_rate)} }; @@ -2004,7 +2090,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 aa4fbec143f..696eab9bd97 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -2224,9 +2224,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 != TERNARY_UNSET) { - if (opts->vacuum_truncate) + if (opts->vacuum_truncate == TERNARY_TRUE) params.truncate = VACOPTVALUE_ENABLED; else params.truncate = VACOPTVALUE_DISABLED; diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h index 2f08e1b0cf0..dfbef2babf2 100644 --- a/src/include/access/reloptions.h +++ b/src/include/access/reloptions.h @@ -29,6 +29,7 @@ typedef enum relopt_type { RELOPT_TYPE_BOOL, + RELOPT_TYPE_TERNARY, /* on, off, unset */ RELOPT_TYPE_INT, RELOPT_TYPE_REAL, RELOPT_TYPE_ENUM, @@ -80,6 +81,7 @@ typedef struct relopt_value union { bool bool_val; + pg_ternary ternary_val; int int_val; double real_val; int enum_val; @@ -94,6 +96,12 @@ typedef struct relopt_bool bool default_val; } relopt_bool; +typedef struct relopt_ternary +{ + relopt_gen gen; + int default_val; +} relopt_ternary; + typedef struct relopt_int { relopt_gen gen; @@ -152,19 +160,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 */ @@ -195,6 +190,9 @@ typedef struct local_relopts 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); +extern void add_ternary_reloption(bits32 kinds, const char *name, + const char *desc, pg_ternary default_val, + LOCKMODE lockmode); extern void add_int_reloption(bits32 kinds, const char *name, const char *desc, int default_val, int min_val, int max_val, LOCKMODE lockmode); @@ -214,6 +212,9 @@ extern void register_reloptions_validator(local_relopts *relopts, extern void add_local_bool_reloption(local_relopts *relopts, const char *name, const char *desc, bool default_val, int offset); +extern void add_local_ternary_reloption(local_relopts *relopts, + const char *name, const char *desc, + pg_ternary default_val, int offset); extern void add_local_int_reloption(local_relopts *relopts, const char *name, const char *desc, int default_val, int min_val, int max_val, int offset); diff --git a/src/include/postgres.h b/src/include/postgres.h index 7d93fbce709..47c88780776 100644 --- a/src/include/postgres.h +++ b/src/include/postgres.h @@ -543,6 +543,21 @@ Float8GetDatum(float8 X) * ---------------------------------------------------------------- */ +/* + * pg_ternary + * Boolean value with an extra "unset" value + * + * This enum can be used for values that want to distinguish between true, + * false, and unset. +*/ + +typedef enum pg_ternary +{ + TERNARY_FALSE = 0, + TERNARY_TRUE = 1, + TERNARY_UNSET = -1 +} pg_ternary; + /* * NON_EXEC_STATIC: It's sometimes useful to define a variable or function * that is normally static but extern when using EXEC_BACKEND (see diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index d03ab247788..5feb18a5373 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -347,8 +347,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 */ + pg_ternary 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/test/regress/expected/reloptions.out b/src/test/regress/expected/reloptions.out index 9de19b4e3f1..1c99f79ab01 100644 --- a/src/test/regress/expected/reloptions.out +++ b/src/test/regress/expected/reloptions.out @@ -98,6 +98,42 @@ SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass; {fillfactor=13,autovacuum_enabled=false} (1 row) +-- Tests for future (FIXME) ternary options +-- behave as boolean option: accept unassigned name and truncated value +DROP TABLE reloptions_test; +CREATE TABLE reloptions_test(i INT) WITH (vacuum_truncate); +SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass; + reloptions +------------------------ + {vacuum_truncate=true} +(1 row) + +DROP TABLE reloptions_test; +CREATE TABLE reloptions_test(i INT) WITH (vacuum_truncate=FaLS); +SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass; + reloptions +------------------------ + {vacuum_truncate=fals} +(1 row) + +-- preferred "true" alias is used when storing +DROP TABLE reloptions_test; +CREATE TABLE reloptions_test(i INT) WITH (vacuum_index_cleanup=on); +SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass; + reloptions +--------------------------- + {vacuum_index_cleanup=on} +(1 row) + +-- custom "third" value is available +DROP TABLE reloptions_test; +CREATE TABLE reloptions_test(i INT) WITH (vacuum_index_cleanup=auto); +SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass; + reloptions +----------------------------- + {vacuum_index_cleanup=auto} +(1 row) + -- Test vacuum_truncate option DROP TABLE reloptions_test; CREATE TEMP TABLE reloptions_test(i INT NOT NULL, j text) diff --git a/src/test/regress/sql/reloptions.sql b/src/test/regress/sql/reloptions.sql index 24fbe0b478d..f5980dafcbc 100644 --- a/src/test/regress/sql/reloptions.sql +++ b/src/test/regress/sql/reloptions.sql @@ -59,6 +59,27 @@ UPDATE pg_class ALTER TABLE reloptions_test RESET (illegal_option); SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass; +-- Tests for future (FIXME) ternary options + +-- behave as boolean option: accept unassigned name and truncated value +DROP TABLE reloptions_test; +CREATE TABLE reloptions_test(i INT) WITH (vacuum_truncate); +SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass; + +DROP TABLE reloptions_test; +CREATE TABLE reloptions_test(i INT) WITH (vacuum_truncate=FaLS); +SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass; + +-- preferred "true" alias is used when storing +DROP TABLE reloptions_test; +CREATE TABLE reloptions_test(i INT) WITH (vacuum_index_cleanup=on); +SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass; + +-- custom "third" value is available +DROP TABLE reloptions_test; +CREATE TABLE reloptions_test(i INT) WITH (vacuum_index_cleanup=auto); +SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass; + -- Test vacuum_truncate option DROP TABLE reloptions_test; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 3f3a888fd0e..1c8610fd46c 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3952,6 +3952,7 @@ pg_sha512_ctx pg_snapshot pg_special_case pg_stack_base_t +pg_ternary pg_time_t pg_time_usec_t pg_tz @@ -4079,6 +4080,7 @@ relopt_kind relopt_parse_elt relopt_real relopt_string +relopt_ternary relopt_type relopt_value relopts_validator -- 2.47.3
