On Sat, May 18, 2019 at 5:55 AM Andres Freund <and...@anarazel.de> wrote: > > Hi, > > On 2019-05-09 14:14:20 +0900, Masahiko Sawada wrote: > > I agree that this item is neither critical and bug. But this is an > > (my) oversight and is a small patch and I think there is no specific > > reason why we don't dare to include this in 12. So if this patch could > > get reviewed enough I think we can have it in 12. Since the previous > > patch conflicts with current HEAD I've attached the rebased version > > patch. > > Robert, this indeed looks near trivial. What do you think? > > > diff --git a/doc/src/sgml/ref/create_table.sgml > > b/doc/src/sgml/ref/create_table.sgml > > index 44a61ef..1e1b0e8 100644 > > --- a/doc/src/sgml/ref/create_table.sgml > > +++ b/doc/src/sgml/ref/create_table.sgml > > @@ -1406,7 +1406,7 @@ WITH ( MODULUS <replaceable > > class="parameter">numeric_literal</replaceable>, REM > > </varlistentry> > > > > <varlistentry id="reloption-vacuum-index-cleanup" > > xreflabel="vacuum_index_cleanup"> > > - <term><literal>vacuum_index_cleanup</literal> (<type>boolean</type>) > > + <term><literal>vacuum_index_cleanup</literal>, > > <literal>toast.vacuum_index_cleanup</literal> (<type>boolean</type>) > > <indexterm> > > <primary><varname>vacuum_index_cleanup</varname> storage > > parameter</primary> > > </indexterm> > > diff --git a/src/backend/access/common/reloptions.c > > b/src/backend/access/common/reloptions.c > > index cfbabb5..022b3a0 100644 > > --- a/src/backend/access/common/reloptions.c > > +++ b/src/backend/access/common/reloptions.c > > @@ -147,7 +147,7 @@ static relopt_bool boolRelOpts[] = > > { > > "vacuum_index_cleanup", > > "Enables index vacuuming and index cleanup", > > - RELOPT_KIND_HEAP, > > + RELOPT_KIND_HEAP | RELOPT_KIND_TOAST, > > ShareUpdateExclusiveLock > > }, > > true > > diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c > > index e4c03de..2379b3d 100644 > > --- a/src/bin/psql/tab-complete.c > > +++ b/src/bin/psql/tab-complete.c > > @@ -1056,6 +1056,7 @@ static const char *const table_storage_parameters[] = > > { > > "toast.autovacuum_vacuum_scale_factor", > > "toast.autovacuum_vacuum_threshold", > > "toast.log_autovacuum_min_duration", > > + "toast.vacuum_index_clenaup", > > "toast.vacuum_truncate", > > "toast_tuple_target", > > "user_catalog_table", > > typo. > > Sawada-san, it'd be good if you could add at least some minimal tests in > the style of the no_index_cleanup test in vacuum.sql. >
Thank you for comments. Attached updated version patch. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index 44a61ef..1e1b0e8 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -1406,7 +1406,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM </varlistentry> <varlistentry id="reloption-vacuum-index-cleanup" xreflabel="vacuum_index_cleanup"> - <term><literal>vacuum_index_cleanup</literal> (<type>boolean</type>) + <term><literal>vacuum_index_cleanup</literal>, <literal>toast.vacuum_index_cleanup</literal> (<type>boolean</type>) <indexterm> <primary><varname>vacuum_index_cleanup</varname> storage parameter</primary> </indexterm> diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c index cfbabb5..022b3a0 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -147,7 +147,7 @@ static relopt_bool boolRelOpts[] = { "vacuum_index_cleanup", "Enables index vacuuming and index cleanup", - RELOPT_KIND_HEAP, + RELOPT_KIND_HEAP | RELOPT_KIND_TOAST, ShareUpdateExclusiveLock }, true diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index e4c03de..e2aa738 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -1056,6 +1056,7 @@ static const char *const table_storage_parameters[] = { "toast.autovacuum_vacuum_scale_factor", "toast.autovacuum_vacuum_threshold", "toast.log_autovacuum_min_duration", + "toast.vacuum_index_cleanup", "toast.vacuum_truncate", "toast_tuple_target", "user_catalog_table", diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out index 8353d84..bc43e82 100644 --- a/src/test/regress/expected/vacuum.out +++ b/src/test/regress/expected/vacuum.out @@ -81,7 +81,10 @@ SQL function "wrap_do_analyze" statement 1 VACUUM FULL vactst; VACUUM (DISABLE_PAGE_SKIPPING) vaccluster; -- INDEX_CLEANUP option -CREATE TABLE no_index_cleanup (i INT PRIMARY KEY) WITH (vacuum_index_cleanup = false); +CREATE TABLE no_index_cleanup (i INT PRIMARY KEY, t TEXT) WITH (vacuum_index_cleanup = false, toast.vacuum_index_cleanup = false); +INSERT INTO no_index_cleanup(i, t) VALUES(1, repeat('1234567890',30000)); +SELECT relname as toastrel FROM pg_class WHERE oid = (SELECT reltoastrelid FROM pg_class WHERE relname = 'no_index_cleanup') \gset +VACUUM pg_toast.:toastrel; -- vacuum toast table of no_index_cleanup with no index cleanup VACUUM (INDEX_CLEANUP FALSE) vaccluster; VACUUM (INDEX_CLEANUP FALSE) vactst; -- index cleanup option is ignored if no indexes VACUUM (INDEX_CLEANUP FALSE, FREEZE TRUE) vaccluster; diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql index a558580..5173539 100644 --- a/src/test/regress/sql/vacuum.sql +++ b/src/test/regress/sql/vacuum.sql @@ -63,7 +63,10 @@ VACUUM FULL vactst; VACUUM (DISABLE_PAGE_SKIPPING) vaccluster; -- INDEX_CLEANUP option -CREATE TABLE no_index_cleanup (i INT PRIMARY KEY) WITH (vacuum_index_cleanup = false); +CREATE TABLE no_index_cleanup (i INT PRIMARY KEY, t TEXT) WITH (vacuum_index_cleanup = false, toast.vacuum_index_cleanup = false); +INSERT INTO no_index_cleanup(i, t) VALUES(1, repeat('1234567890',30000)); +SELECT relname as toastrel FROM pg_class WHERE oid = (SELECT reltoastrelid FROM pg_class WHERE relname = 'no_index_cleanup') \gset +VACUUM pg_toast.:toastrel; -- vacuum toast table of no_index_cleanup with no index cleanup VACUUM (INDEX_CLEANUP FALSE) vaccluster; VACUUM (INDEX_CLEANUP FALSE) vactst; -- index cleanup option is ignored if no indexes VACUUM (INDEX_CLEANUP FALSE, FREEZE TRUE) vaccluster;