From: Tom Lane Date: Sat, 10 Aug 2019 15:30:12 +0000 (-0400) Subject: Fix "ANALYZE t, t" inside a transaction block. X-Git-Tag: REL_11_6~133 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ceb850d4a30c06f2f127487acd898847b80dcfc5;p=thirdparty%2Fpostgresql.git Fix "ANALYZE t, t" inside a transaction block. This failed with either "tuple already updated by self" or "duplicate key value violates unique constraint", depending on whether the table had previously been analyzed or not. The reason is that ANALYZE tried to insert or update the same pg_statistic rows twice, and there was no CommandCounterIncrement between. So add one. The same case works fine outside a transaction block, because then there's a whole transaction boundary between, as a consequence of the way VACUUM works. This issue has been latent all along, but the problem was unreachable before commit 11d8d72c2 added the ability to specify multiple tables in ANALYZE. We could, perhaps, alternatively fix it by adding code to de-duplicate the list of VacuumRelations --- but that would add a lot of overhead to work around dumb commands, so it's not attractive. Per bug #15946 from Yaroslav Schekin. Back-patch to v11. (Note: in v11 I also back-patched the test added by commit 23224563d; otherwise the problem doesn't manifest in the test I added, because "vactst" is empty when the tests for multiple ANALYZE targets are reached. That seems like not a very good thing anyway, so I did this rather than rethinking the choice of test case.) Discussion: https://postgr.es/m/15946-5c7570a2884a26cf@postgresql.org --- diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index d90cb9a9022..48538fdc9ad 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -362,6 +362,15 @@ vacuum(int options, List *relations, VacuumParams *params, PopActiveSnapshot(); CommitTransactionCommand(); } + else + { + /* + * If we're not using separate xacts, better separate the + * ANALYZE actions with CCIs. This avoids trouble if user + * says "ANALYZE t, t". + */ + CommandCounterIncrement(); + } } } } diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out index d66e2aa3b70..b6e0cca6979 100644 --- a/src/test/regress/expected/vacuum.out +++ b/src/test/regress/expected/vacuum.out @@ -71,6 +71,18 @@ ANALYZE vaccluster; ERROR: ANALYZE cannot be executed from VACUUM or ANALYZE CONTEXT: SQL function "do_analyze" statement 1 SQL function "wrap_do_analyze" statement 1 +-- Test ANALYZE in transaction, where the transaction surrounding +-- analyze performed modifications. This tests for the bug at +-- https://postgr.es/m/c7988239-d42c-ddc4-41db-171b23b35e4f%40ssinger.info +-- (which hopefully is unlikely to be reintroduced), but also seems +-- independently worthwhile to cover. +INSERT INTO vactst SELECT generate_series(1, 300); +DELETE FROM vactst WHERE i % 7 = 0; -- delete a few rows outside +BEGIN; +INSERT INTO vactst SELECT generate_series(301, 400); +DELETE FROM vactst WHERE i % 5 <> 0; -- delete a few rows inside +ANALYZE vactst; +COMMIT; VACUUM FULL pg_am; VACUUM FULL pg_class; VACUUM FULL pg_database; @@ -112,6 +124,10 @@ ANALYZE vactst, does_not_exist, vacparted; ERROR: relation "does_not_exist" does not exist ANALYZE vactst (i), vacparted (does_not_exist); ERROR: column "does_not_exist" of relation "vacparted" does not exist +ANALYZE vactst, vactst; +BEGIN; -- ANALYZE behaves differently inside a transaction block +ANALYZE vactst, vactst; +COMMIT; -- parenthesized syntax for ANALYZE ANALYZE (VERBOSE) does_not_exist; ERROR: relation "does_not_exist" does not exist diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql index 275ce2e270f..2e2338db60c 100644 --- a/src/test/regress/sql/vacuum.sql +++ b/src/test/regress/sql/vacuum.sql @@ -54,6 +54,19 @@ CREATE INDEX ON vaccluster(wrap_do_analyze(i)); INSERT INTO vaccluster VALUES (1), (2); ANALYZE vaccluster; +-- Test ANALYZE in transaction, where the transaction surrounding +-- analyze performed modifications. This tests for the bug at +-- https://postgr.es/m/c7988239-d42c-ddc4-41db-171b23b35e4f%40ssinger.info +-- (which hopefully is unlikely to be reintroduced), but also seems +-- independently worthwhile to cover. +INSERT INTO vactst SELECT generate_series(1, 300); +DELETE FROM vactst WHERE i % 7 = 0; -- delete a few rows outside +BEGIN; +INSERT INTO vactst SELECT generate_series(301, 400); +DELETE FROM vactst WHERE i % 5 <> 0; -- delete a few rows inside +ANALYZE vactst; +COMMIT; + VACUUM FULL pg_am; VACUUM FULL pg_class; VACUUM FULL pg_database; @@ -88,6 +101,10 @@ ANALYZE vactst, vacparted; ANALYZE vacparted (b), vactst; ANALYZE vactst, does_not_exist, vacparted; ANALYZE vactst (i), vacparted (does_not_exist); +ANALYZE vactst, vactst; +BEGIN; -- ANALYZE behaves differently inside a transaction block +ANALYZE vactst, vactst; +COMMIT; -- parenthesized syntax for ANALYZE ANALYZE (VERBOSE) does_not_exist;