agora inbox for pgsql-hackers@postgresql.org  
help / color / mirror / Atom feed
From: Álvaro Herrera <alvherre@kurilemu.de>
To: Chao Li <li.evan.chao@gmail.com>
Cc: Baji Shaik <baji.pgdev@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Improve REPACK (CONCURRENTLY) error messages for unsupported configurations
Date: Thu, 28 May 2026 23:18:08 +0200
Message-ID: <ahiwD29RNfVT4tjQ@alvherre.pgsql> (raw)
In-Reply-To: <ahiKMDxJ-G4bAy8Q@alvherre.pgsql>

While looking these patches over I noticed that we still have some error
reports cases uncovered.  Here's a quick attempt to try and complete
that.

After this patch I see only one uncovered error path, the one that
prevents repacking a temp table of another session.  That would require
an isolation test.  Not sure it's worth the trouble ...  (There's a
bunch of uncovered "elog(ERROR)" cases, but those are mostly just
can't-happen conditions, as I understand).

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/

Attachments:

  [text/x-diff] 0001-Cover-some-errors-and-corner-conditions-in-repack.c.patch (4.0K, ../ahiwD29RNfVT4tjQ@alvherre.pgsql/2-0001-Cover-some-errors-and-corner-conditions-in-repack.c.patch)
  download | inline diff:
From 7eab30e74692ac023ed20bfc85d92e68f0d9db02 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Thu, 28 May 2026 11:20:19 +0200
Subject: [PATCH] Cover some errors and corner conditions in repack.c

---
 src/test/regress/expected/cluster.out | 33 +++++++++++++++++++++++++++
 src/test/regress/sql/cluster.sql      | 28 +++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index 23f312c62a3..cfbe2764427 100644
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -697,6 +697,39 @@ SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b;
 (4 rows)
 
 COMMIT;
+-- verify some error cases
+CREATE TABLE clstr_table_one (id int, val text);
+CREATE TABLE clstr_table_two (id int, val text);
+CREATE INDEX clstr_idx_b ON clstr_table_two (id);
+CLUSTER clstr_table_one USING clstr_idx_b;
+ERROR:  "clstr_idx_b" is not an index for table "clstr_table_one"
+CLUSTER clstr_table_one USING nonexistant;
+ERROR:  index "nonexistant" for table "clstr_table_one" does not exist
+CREATE INDEX clstr_hash_idx ON clstr_table_one USING hash (id);
+CLUSTER clstr_table_one USING clstr_hash_idx;
+ERROR:  cannot cluster on index "clstr_hash_idx" because access method does not support clustering
+CREATE INDEX clstr_partial_idx ON clstr_table_one (id) WHERE id > 0;
+CLUSTER clstr_table_one USING clstr_partial_idx;
+ERROR:  cannot cluster on partial index "clstr_partial_idx"
+REPACK pg_class USING INDEX pg_class_oid_index;
+ERROR:  permission denied: "pg_class" is a system catalog
+DETAIL:  System catalogs can only be clustered by the index they're already clustered on, if any, unless "allow_system_table_mods" is enabled.
+DROP TABLE clstr_table_one, clstr_table_two;
+-- verify that CLUSTER/REPACK don't touch a NO DATA matview
+CREATE MATERIALIZED VIEW clstr_matview AS
+    SELECT i FROM generate_series(1, 5) i
+    WITH NO DATA;
+CREATE INDEX clstr_matview_idx ON clstr_matview (i);
+SELECT relfilenode FROM pg_class WHERE oid = 'clstr_matview'::regclass \gset
+CLUSTER clstr_matview USING clstr_matview_idx;
+REPACK clstr_matview USING INDEX clstr_matview_idx;
+SELECT relfilenode = :relfilenode FROM pg_class WHERE oid = 'clstr_matview'::regclass;
+ ?column? 
+----------
+ t
+(1 row)
+
+DROP MATERIALIZED VIEW clstr_matview;
 ----------------------------------------------------------------------
 --
 -- REPACK
diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql
index c2f329ecd1b..1cb1942263c 100644
--- a/src/test/regress/sql/cluster.sql
+++ b/src/test/regress/sql/cluster.sql
@@ -328,6 +328,34 @@ EXPLAIN (COSTS OFF) SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b;
 SELECT * FROM clstr_expression WHERE -a = -3 ORDER BY -a, b;
 COMMIT;
 
+-- verify some error cases
+CREATE TABLE clstr_table_one (id int, val text);
+CREATE TABLE clstr_table_two (id int, val text);
+CREATE INDEX clstr_idx_b ON clstr_table_two (id);
+CLUSTER clstr_table_one USING clstr_idx_b;
+CLUSTER clstr_table_one USING nonexistant;
+
+CREATE INDEX clstr_hash_idx ON clstr_table_one USING hash (id);
+CLUSTER clstr_table_one USING clstr_hash_idx;
+
+CREATE INDEX clstr_partial_idx ON clstr_table_one (id) WHERE id > 0;
+CLUSTER clstr_table_one USING clstr_partial_idx;
+
+REPACK pg_class USING INDEX pg_class_oid_index;
+
+DROP TABLE clstr_table_one, clstr_table_two;
+
+-- verify that CLUSTER/REPACK don't touch a NO DATA matview
+CREATE MATERIALIZED VIEW clstr_matview AS
+    SELECT i FROM generate_series(1, 5) i
+    WITH NO DATA;
+CREATE INDEX clstr_matview_idx ON clstr_matview (i);
+SELECT relfilenode FROM pg_class WHERE oid = 'clstr_matview'::regclass \gset
+CLUSTER clstr_matview USING clstr_matview_idx;
+REPACK clstr_matview USING INDEX clstr_matview_idx;
+SELECT relfilenode = :relfilenode FROM pg_class WHERE oid = 'clstr_matview'::regclass;
+DROP MATERIALIZED VIEW clstr_matview;
+
 ----------------------------------------------------------------------
 --
 -- REPACK
-- 
2.47.3

view thread (9+ messages)  latest in thread

Message-ID: <ahiwD29RNfVT4tjQ@alvherre.pgsql>
Permalink:  ../ahiwD29RNfVT4tjQ@alvherre.pgsql/
Also on:    postgresql.org/message-id/ahiwD29RNfVT4tjQ@alvherre.pgsql

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: alvherre@kurilemu.de, li.evan.chao@gmail.com, baji.pgdev@gmail.com, pgsql-hackers@lists.postgresql.org
  Subject: Re: [PATCH] Improve REPACK (CONCURRENTLY) error messages for unsupported configurations
  In-Reply-To: <ahiwD29RNfVT4tjQ@alvherre.pgsql>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox