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;
