REPACK CONCURRENTLY: Don't use deferrable primary keys

Similarly to logical replication, REPACK CONCURRENTLY needs to ability
to reliably locate a tuple based on an identity.  A replica identity
index is okay.  Primary keys normally also are, except when they are
deferrable, because a tuple being modified might not yet be indexed,
causing REPACK to fail.

Change the REPACK CONCURRENTLY code to use GetRelationIdentityOrPK(),
similar to what the logical replication code does.  (Though we don't yet
support locating tuples based on arbitrary indexes for replica identity
FULL.)

While at it, add a few more test cases for situations that aren't
supported by REPACK, to improve coverage.

Author: Chao Li <lic@highgo.com>
Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Reviewed-by: Yuchen Li <liyuchen_xyz@163.com>
Discussion: https://postgr.es/m/10DD5E13-B45D-44F1-BE08-C63E00ABCAC0@gmail.com

diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index bafdca80810ec126ae92117e630742d62aef5d75..ce5b99c38b1e5f62e1f2b29d919fcd97b214a66f 100644 (file)
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -62,6 +62,7 @@
 #include "miscadmin.h"
 #include "optimizer/optimizer.h"
 #include "pgstat.h"
+#include "replication/logicalrelation.h"
 #include "storage/bufmgr.h"
 #include "storage/lmgr.h"
 #include "storage/predicate.h"
@@ -919,14 +920,12 @@ check_concurrent_repack_requirements(Relation rel, Oid *ident_idx_p)
 
        /*
         * Obtain the replica identity index -- either one that has been set
-        * explicitly, or the primary key.  If none of these cases apply, the
-        * table cannot be repacked concurrently.  It might be possible to have
-        * repack work with a FULL replica identity; however that requires more
-        * work and is not implemented yet.
-        */
-       ident_idx = RelationGetReplicaIndex(rel);
-       if (!OidIsValid(ident_idx) && OidIsValid(rel->rd_pkindex))
-               ident_idx = rel->rd_pkindex;
+        * explicitly, or a non-deferrable primary key.  If none of these cases
+        * apply, the table cannot be repacked concurrently.  It might be possible
+        * to have repack work with a FULL replica identity; however that requires
+        * more work and is not implemented yet.
+        */
+       ident_idx = GetRelationIdentityOrPK(rel);
        if (!OidIsValid(ident_idx))
                ereport(ERROR,
                                errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),

diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index 96089bb0fa20ecc9d8547a2008c020ec1e5f9bdc..8f80f9f752d7d6f853a9dd4497e505400c5e3298 100644 (file)
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -796,15 +796,63 @@ ORDER BY o.relname;
  clstr_3
 (2 rows)
 
--- concurrently disallowed in catalogs
+--
+-- Check concurrent mode requirements
+--
+-- Disallowed in catalogs
 REPACK (CONCURRENTLY) pg_class;
 ERROR:  cannot repack relation "pg_class"
 HINT:  REPACK CONCURRENTLY is not supported for catalog relations.
--- CONCURRENTLY doesn't like partitioned tables
+-- Doesn't like partitioned tables
 REPACK (CONCURRENTLY) clstrpart;
 ERROR:  REPACK (CONCURRENTLY) is not supported for partitioned tables
 HINT:  Consider running the command on individual partitions.
+-- Doesn't support catalog tables
+REPACK (CONCURRENTLY) pg_class;
+ERROR:  cannot repack relation "pg_class"
+HINT:  REPACK CONCURRENTLY is not supported for catalog relations.
+-- Only support permanent tables, temp and unlogged tables are not supported
+CREATE TEMP TABLE repack_conc_temp (i int PRIMARY KEY);
+REPACK (CONCURRENTLY) repack_conc_temp;
+ERROR:  cannot repack relation "repack_conc_temp"
+HINT:  REPACK CONCURRENTLY is only allowed for permanent relations.
+DROP TABLE repack_conc_temp;
+CREATE UNLOGGED TABLE repack_conc_unlogged (i int PRIMARY KEY);
+REPACK (CONCURRENTLY) repack_conc_unlogged;
+ERROR:  cannot repack relation "repack_conc_unlogged"
+HINT:  REPACK CONCURRENTLY is only allowed for permanent relations.
+DROP TABLE repack_conc_unlogged;
+-- Doesn't support TOAST tables directly
+CREATE TABLE repack_conc_toast (t text);
+SELECT reltoastrelid::regclass AS toast_rel
+FROM pg_class WHERE oid = 'repack_conc_toast'::regclass \gset
+\set VERBOSITY sqlstate
+REPACK (CONCURRENTLY) :toast_rel;
+ERROR:  0A000
+\set VERBOSITY default
+DROP TABLE repack_conc_toast;
+-- Doesn't support tables with REPLICA IDENTITY NOTHING, even if they have a primary key
+CREATE TABLE repack_conc_replident (i int PRIMARY KEY);
+ALTER TABLE repack_conc_replident REPLICA IDENTITY NOTHING;
+REPACK (CONCURRENTLY) repack_conc_replident;
+ERROR:  cannot repack relation "repack_conc_replident"
+HINT:  Relation "repack_conc_replident" has insufficient replication identity.
+-- Doesn't support tables with REPLICA IDENTITY FULL, even if they have a primary key
+ALTER TABLE repack_conc_replident REPLICA IDENTITY FULL;
+REPACK (CONCURRENTLY) repack_conc_replident;
+-- Doesn't support tables without a primary key or replica identity index
+ALTER TABLE repack_conc_replident DROP CONSTRAINT repack_conc_replident_pkey;
+ALTER TABLE repack_conc_replident REPLICA IDENTITY DEFAULT;
+REPACK (CONCURRENTLY) repack_conc_replident;
+ERROR:  cannot process relation "repack_conc_replident"
+HINT:  Relation "repack_conc_replident" has no identity index.
+-- Doesn't support tables with deferrable primary keys
+ALTER TABLE repack_conc_replident ADD PRIMARY KEY (i) DEFERRABLE;
+REPACK (CONCURRENTLY) repack_conc_replident;
+ERROR:  cannot process relation "repack_conc_replident"
+HINT:  Relation "repack_conc_replident" has no identity index.
 -- clean up
+DROP TABLE repack_conc_replident;
 DROP TABLE clustertest;
 DROP TABLE clstr_1;
 DROP TABLE clstr_2;

diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql
index 6b3219bab94c69fe4ab894d68f75a68bf2b6b563..a8cb7ca982dea6cfa8512bbe6f25e1383ff7b4c8 100644 (file)
--- a/src/test/regress/sql/cluster.sql
+++ b/src/test/regress/sql/cluster.sql
@@ -383,13 +383,56 @@ JOIN relnodes_new n ON o.relname = n.relname
 WHERE o.relfilenode <> n.relfilenode
 ORDER BY o.relname;
 
--- concurrently disallowed in catalogs
+--
+-- Check concurrent mode requirements
+--
+
+-- Disallowed in catalogs
 REPACK (CONCURRENTLY) pg_class;
 
--- CONCURRENTLY doesn't like partitioned tables
+-- Doesn't like partitioned tables
 REPACK (CONCURRENTLY) clstrpart;
 
+-- Doesn't support catalog tables
+REPACK (CONCURRENTLY) pg_class;
+
+-- Only support permanent tables, temp and unlogged tables are not supported
+CREATE TEMP TABLE repack_conc_temp (i int PRIMARY KEY);
+REPACK (CONCURRENTLY) repack_conc_temp;
+DROP TABLE repack_conc_temp;
+CREATE UNLOGGED TABLE repack_conc_unlogged (i int PRIMARY KEY);
+REPACK (CONCURRENTLY) repack_conc_unlogged;
+DROP TABLE repack_conc_unlogged;
+
+-- Doesn't support TOAST tables directly
+CREATE TABLE repack_conc_toast (t text);
+SELECT reltoastrelid::regclass AS toast_rel
+FROM pg_class WHERE oid = 'repack_conc_toast'::regclass \gset
+\set VERBOSITY sqlstate
+REPACK (CONCURRENTLY) :toast_rel;
+\set VERBOSITY default
+DROP TABLE repack_conc_toast;
+
+-- Doesn't support tables with REPLICA IDENTITY NOTHING, even if they have a primary key
+CREATE TABLE repack_conc_replident (i int PRIMARY KEY);
+ALTER TABLE repack_conc_replident REPLICA IDENTITY NOTHING;
+REPACK (CONCURRENTLY) repack_conc_replident;
+
+-- Doesn't support tables with REPLICA IDENTITY FULL, even if they have a primary key
+ALTER TABLE repack_conc_replident REPLICA IDENTITY FULL;
+REPACK (CONCURRENTLY) repack_conc_replident;
+
+-- Doesn't support tables without a primary key or replica identity index
+ALTER TABLE repack_conc_replident DROP CONSTRAINT repack_conc_replident_pkey;
+ALTER TABLE repack_conc_replident REPLICA IDENTITY DEFAULT;
+REPACK (CONCURRENTLY) repack_conc_replident;
+
+-- Doesn't support tables with deferrable primary keys
+ALTER TABLE repack_conc_replident ADD PRIMARY KEY (i) DEFERRABLE;
+REPACK (CONCURRENTLY) repack_conc_replident;
+
 -- clean up
+DROP TABLE repack_conc_replident;
 DROP TABLE clustertest;
 DROP TABLE clstr_1;
 DROP TABLE clstr_2;