On Wed, Jun 19, 2019 at 12:51:41PM -0700, Peter Geoghegan wrote:
> On Tue, Jun 18, 2019 at 10:39 PM Michael Paquier <mich...@paquier.xyz> wrote:
>> +INSERT INTO no_index_cleanup(i, t) VALUES(1, repeat('1234567890',30000));
>> Do we really need a string as long as that?
>
> Specifying EXTERNAL storage might make things easier. I have used
> PLAIN storage to test the 1/3 of a page restriction within nbtree, and
> to test a bug in amcheck that was related to TOAST compression.Ah, good point here. That makes sense. >> It seems to me that we'd want tests to make sure that indexes are >> actually cleaned up, where pageinspect could prove to be useful. > > That definitely seems preferable, but it'll be a bit tricky to do it > in a way that doesn't run into buildfarm issues due to alignment. I > suggest an index on a text column to avoid problems. I am not completely sure how tricky that may be, so I'll believe you on this one :) So, to keep things simple and if we want to get something into v12, I would suggest to just stress more combinations even if the changes are not entirely visible yet. If we get a couple of queries to run with the option disabled on the table, its toast or both by truncating and filling in the table in-between, we may be able to catch some issues by stressing those code paths. And I finish with the attached. Thoughts? -- Michael
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 8e1bfe4d78..9009addb9c 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 de06c92574..5773021499 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -144,7 +144,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 5e38f46399..7dcf342413 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1069,6 +1069,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 f944b93fd6..5d50548f65 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -93,13 +93,38 @@ 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);
-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;
+CREATE TABLE no_index_cleanup (i INT PRIMARY KEY, t TEXT);
+-- Use uncompressed data stored in toast.
+CREATE INDEX no_index_cleanup_idx ON no_index_cleanup(t);
+ALTER TABLE no_index_cleanup ALTER COLUMN t SET STORAGE EXTERNAL;
+INSERT INTO no_index_cleanup(i, t) VALUES (generate_series(1,30),
+ repeat('1234567890',300));
-- index cleanup option is ignored if VACUUM FULL
VACUUM (INDEX_CLEANUP TRUE, FULL TRUE) no_index_cleanup;
VACUUM (FULL TRUE) no_index_cleanup;
+-- Toast inherits the value from its parent table.
+DELETE FROM no_index_cleanup WHERE i < 15;
+-- Nothing is cleaned up.
+ALTER TABLE no_index_cleanup SET (vacuum_index_cleanup = false);
+-- Both parent relation and toast are cleaned up.
+ALTER TABLE no_index_cleanup SET (vacuum_index_cleanup = true);
+VACUUM no_index_cleanup;
+-- Parameter is set for both the parent table and its toast relation.
+INSERT INTO no_index_cleanup(i, t) VALUES (generate_series(31,60),
+ repeat('1234567890',300));
+DELETE FROM no_index_cleanup WHERE i < 45;
+-- Only toast index is cleaned up.
+ALTER TABLE no_index_cleanup SET (vacuum_index_cleanup = false,
+ toast.vacuum_index_cleanup = true);
+VACUUM no_index_cleanup;
+-- Only parent is cleaned up.
+ALTER TABLE no_index_cleanup SET (vacuum_index_cleanup = true,
+ toast.vacuum_index_cleanup = false);
+VACUUM no_index_cleanup;
+-- Test some extra relations.
+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;
-- TRUNCATE option
CREATE TABLE vac_truncate_test(i INT NOT NULL, j text)
WITH (vacuum_truncate=true, autovacuum_enabled=false);
diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql
index 16748e1823..91a2ea6482 100644
--- a/src/test/regress/sql/vacuum.sql
+++ b/src/test/regress/sql/vacuum.sql
@@ -76,13 +76,38 @@ 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);
-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;
+CREATE TABLE no_index_cleanup (i INT PRIMARY KEY, t TEXT);
+-- Use uncompressed data stored in toast.
+CREATE INDEX no_index_cleanup_idx ON no_index_cleanup(t);
+ALTER TABLE no_index_cleanup ALTER COLUMN t SET STORAGE EXTERNAL;
+INSERT INTO no_index_cleanup(i, t) VALUES (generate_series(1,30),
+ repeat('1234567890',300));
-- index cleanup option is ignored if VACUUM FULL
VACUUM (INDEX_CLEANUP TRUE, FULL TRUE) no_index_cleanup;
VACUUM (FULL TRUE) no_index_cleanup;
+-- Toast inherits the value from its parent table.
+DELETE FROM no_index_cleanup WHERE i < 15;
+-- Nothing is cleaned up.
+ALTER TABLE no_index_cleanup SET (vacuum_index_cleanup = false);
+-- Both parent relation and toast are cleaned up.
+ALTER TABLE no_index_cleanup SET (vacuum_index_cleanup = true);
+VACUUM no_index_cleanup;
+-- Parameter is set for both the parent table and its toast relation.
+INSERT INTO no_index_cleanup(i, t) VALUES (generate_series(31,60),
+ repeat('1234567890',300));
+DELETE FROM no_index_cleanup WHERE i < 45;
+-- Only toast index is cleaned up.
+ALTER TABLE no_index_cleanup SET (vacuum_index_cleanup = false,
+ toast.vacuum_index_cleanup = true);
+VACUUM no_index_cleanup;
+-- Only parent is cleaned up.
+ALTER TABLE no_index_cleanup SET (vacuum_index_cleanup = true,
+ toast.vacuum_index_cleanup = false);
+VACUUM no_index_cleanup;
+-- Test some extra relations.
+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;
-- TRUNCATE option
CREATE TABLE vac_truncate_test(i INT NOT NULL, j text)
signature.asc
Description: PGP signature
