REPACK (CONCURRENTLY) rewrites tables marked with user_catalog_table

Hi,

On Thu, Aug 27, 2026 at 7:22 AM Nathan Bossart <nathandbossart@gmail.com> wrote:

IIUC this can cause problems for logical replication consumers, since the
CONCURRENTLY path doesn't go through the rewriteheap.c machinery like the
non-concurrent path does:

CREATE TABLE t (a INT PRIMARY KEY, b TEXT) WITH (user_catalog_table = true);
ALTER TABLE t ALTER COLUMN b TYPE varchar(100); -- correctly fails
REPACK (CONCURRENTLY) t; -- should fail but doesn't

I believe the fix for v19 is to add a check for
RelationIsUsedAsCatalogTable() in check_concurrent_repack_requirements()
(plus a corresponding update to the list of CONCURRENTLY limitations in the
docs).

That's correct. I verified that concurrent repack doesn't write the
pg_logical/mappings files, which decoding later needs to correctly map
the old tuple locations to the new ones after a rewrite, unlike
non-concurrent CLUSTER, VACUUM FULL and REPACK on a user catalog
table. I think restricting user catalog tables, rather than making
concurrent repack support them, is the right choice at this point in
the release cycle.

Please find attached a patch with the suggested doc change. I didn't
add a test, I don't think we need one.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:

Hi,

On Thu, Aug 27, 2026 at 7:22 AM Nathan Bossart <nathandbossart@gmail.com> wrote:

IIUC this can cause problems for logical replication consumers, since the
CONCURRENTLY path doesn't go through the rewriteheap.c machinery like the
non-concurrent path does:

CREATE TABLE t (a INT PRIMARY KEY, b TEXT) WITH (user_catalog_table = true);
ALTER TABLE t ALTER COLUMN b TYPE varchar(100); -- correctly fails
REPACK (CONCURRENTLY) t; -- should fail but doesn't

I believe the fix for v19 is to add a check for
RelationIsUsedAsCatalogTable() in check_concurrent_repack_requirements()
(plus a corresponding update to the list of CONCURRENTLY limitations in the
docs).

That's correct. I verified that concurrent repack doesn't write the
pg_logical/mappings files, which decoding later needs to correctly map
the old tuple locations to the new ones after a rewrite, unlike
non-concurrent CLUSTER, VACUUM FULL and REPACK on a user catalog
table. I think restricting user catalog tables, rather than making
concurrent repack support them, is the right choice at this point in
the release cycle.

Please find attached a patch with the suggested doc change. I didn't
add a test, I don't think we need one.

Thanks! I'm just thinking about this comment:

+ /* The CONCURRENTLY path does not write logical rewrite mappings. */

IMO it makes no sense to rewrite the logical mappings as long as REPACK
(CONCURRENTLY) changes visibility information (i.e. it's MVCC-unsafe). Once we
implement the MVCC-safety, we should not remove this check unless we implement
the rewriting of the mappings for user catalog tables - that might also be
worth mentioning in the comment.

So far, I'd consider the MVCC-unsafety the primary reason to prohibit REPACK
(CONCURRENTLY) (or anyother MVCC-unsafe command) on the user catalog tables,
because it can make the contents of those tables invisible to the output
plugin at some point.

--
Antonin Houska
Web: https://www.cybertec-postgresql.com

Hi,

On Sat, Aug 29, 2026 at 4:23 AM Antonin Houska <ah@cybertec.at> wrote:

Thanks! I'm just thinking about this comment:

+ /* The CONCURRENTLY path does not write logical rewrite mappings. */

IMO it makes no sense to rewrite the logical mappings as long as REPACK
(CONCURRENTLY) changes visibility information (i.e. it's MVCC-unsafe). Once we
implement the MVCC-safety, we should not remove this check unless we implement
the rewriting of the mappings for user catalog tables - that might also be
worth mentioning in the comment.

So far, I'd consider the MVCC-unsafety the primary reason to prohibit REPACK
(CONCURRENTLY) (or anyother MVCC-unsafe command) on the user catalog tables,
because it can make the contents of those tables invisible to the output
plugin at some point.

Thanks for pointing this out. I read the docs related to MVCC-safety
of concurrent repack. I reworded the comment as you suggested. Feel
free to suggest changes to it if any. Please find the attached v2
patch.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

Hi,

On Sat, Aug 29, 2026 at 8:34 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

Hi,

On Sat, Aug 29, 2026 at 4:23 AM Antonin Houska <ah@cybertec.at> wrote:

Thanks! I'm just thinking about this comment:

+ /* The CONCURRENTLY path does not write logical rewrite mappings. */

IMO it makes no sense to rewrite the logical mappings as long as REPACK
(CONCURRENTLY) changes visibility information (i.e. it's MVCC-unsafe). Once we
implement the MVCC-safety, we should not remove this check unless we implement
the rewriting of the mappings for user catalog tables - that might also be
worth mentioning in the comment.

So far, I'd consider the MVCC-unsafety the primary reason to prohibit REPACK
(CONCURRENTLY) (or anyother MVCC-unsafe command) on the user catalog tables,
because it can make the contents of those tables invisible to the output
plugin at some point.

Thanks for pointing this out. I read the docs related to MVCC-safety
of concurrent repack. I reworded the comment as you suggested. Feel
free to suggest changes to it if any. Please find the attached v2
patch.

Please find the attached v3 patch with a test added similar to other
restricted error cases. Sorry for the noise.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

On Mon, Aug 31, 2026 at 8:35 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

Hi,

On Sat, Aug 29, 2026 at 8:34 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

Hi,

On Sat, Aug 29, 2026 at 4:23 AM Antonin Houska <ah@cybertec.at> wrote:

Thanks! I'm just thinking about this comment:

+ /* The CONCURRENTLY path does not write logical rewrite mappings. */

IMO it makes no sense to rewrite the logical mappings as long as REPACK
(CONCURRENTLY) changes visibility information (i.e. it's MVCC-unsafe). Once we
implement the MVCC-safety, we should not remove this check unless we implement
the rewriting of the mappings for user catalog tables - that might also be
worth mentioning in the comment.

So far, I'd consider the MVCC-unsafety the primary reason to prohibit REPACK
(CONCURRENTLY) (or anyother MVCC-unsafe command) on the user catalog tables,
because it can make the contents of those tables invisible to the output
plugin at some point.

Thanks for pointing this out. I read the docs related to MVCC-safety
of concurrent repack. I reworded the comment as you suggested. Feel
free to suggest changes to it if any. Please find the attached v2
patch.

Please find the attached v3 patch with a test added similar to other
restricted error cases. Sorry for the noise.

I reviewed the patch. It's simple and I agree to add a check for user
catalog tables. I've slightly rephrased the comment in
check_concurrent_repack_requirements() to clarify the reason why
REPACK (CONCURRENTLY) doesn't support user catalog tables while it
does for normal tables even though it's a MVCC-unsafe operation. Also
updated the commit message as well. Please review it.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Hi,

On Fri, Sep 4, 2026 at 11:02 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

I reviewed the patch. It's simple and I agree to add a check for user
catalog tables. I've slightly rephrased the comment in
check_concurrent_repack_requirements() to clarify the reason why
REPACK (CONCURRENTLY) doesn't support user catalog tables while it
does for normal tables even though it's a MVCC-unsafe operation. Also
updated the commit message as well. Please review it.

Thanks for reviewing it. The v4 patch looks good to me. pgindent and
make check are happy.

A nit:

Code comment:
+ * tables under a historic snapshot. Removing this check requires making
+ * it MVCC-safe and logical rewrite mappings.
+ */

Commit message:
Disallow REPACK (CONCURRENTLY) on user catalog tables. Removing this
check requires making it MVCC-safe and logical rewrite mappings.

"Making it logical rewrite mappings" doesn't read correctly. How about
"Removing this check requires making it MVCC-safe and adding logical
rewrite mapping support" or such?

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

Hello,

On 2026-Sep-04, Masahiko Sawada wrote:

I reviewed the patch. It's simple and I agree to add a check for user
catalog tables. I've slightly rephrased the comment in
check_concurrent_repack_requirements() to clarify the reason why
REPACK (CONCURRENTLY) doesn't support user catalog tables while it
does for normal tables even though it's a MVCC-unsafe operation. Also
updated the commit message as well. Please review it.

Thanks, I gave this a look and I think it's okay. I'm not sure about
the exact wording of the error message though,

+		ereport(ERROR,
+				errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				errmsg("cannot execute %s on relation \"%s\"",
+					   "REPACK (CONCURRENTLY)", RelationGetRelationName(rel)),
+				errhint("%s is not supported for tables used as catalog tables.",
+						"REPACK (CONCURRENTLY)"));

ERROR: cannot execute REPACK (CONCURRENTLY) on relation "foobar"
HINT: REPACK (CONCURRENTLY) is not supported for tables used as catalog tables.

I think the HINT should really be DETAIL, and I'm not sure we need to
mention the command again there. On the other hand, I'm not really
happy with the term "tables used as catalog tables". In the
documentation we use the term "user provided catalog tables" or simply
"user catalog tables" (see <sect2 id="logicaldecoding-capabilities">).
So how about something like this?

ERROR: cannot execute REPACK (CONCURRENTLY) on relation "foobar"
DETAIL: This operation is not supported for user catalog tables.

Would you push this patch, or do you want me to?

Thanks!

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/

Hi,

On Tue, Sep 8, 2026 at 5:47 AM Alvaro Herrera <alvherre@kurilemu.de> wrote:

Thanks, I gave this a look and I think it's okay. I'm not sure about
the exact wording of the error message though,

Thanks for reviewing it.

+             ereport(ERROR,
+                             errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                             errmsg("cannot execute %s on relation \"%s\"",
+                                        "REPACK (CONCURRENTLY)", RelationGetRelationName(rel)),
+                             errhint("%s is not supported for tables used as catalog tables.",
+                                             "REPACK (CONCURRENTLY)"));

ERROR: cannot execute REPACK (CONCURRENTLY) on relation "foobar"
HINT: REPACK (CONCURRENTLY) is not supported for tables used as catalog tables.

I think the HINT should really be DETAIL, and I'm not sure we need to
mention the command again there.

On the other hand, I'm not really
happy with the term "tables used as catalog tables". In the
documentation we use the term "user provided catalog tables" or simply
"user catalog tables" (see <sect2 id="logicaldecoding-capabilities">).
So how about something like this?

ERROR: cannot execute REPACK (CONCURRENTLY) on relation "foobar"
DETAIL: This operation is not supported for user catalog tables.

I agree that this is not a hint message. If we ever say something like
"Please do something to get repack to work on user catalog tables,"
that would be a hint, but here we are giving more details about the
error itself. And "user catalog tables" looks good to me too (although
existing error messages around RelationIsUsedAsCatalogTable() uses
table used as a catalog table, with "system catalogs" meaning the
core-provided ones. Also, the proposed wording looks consistent with
errdetail_relkind_not_supported(). So, I used that in the attached
0001.

I agree that this is not a hint message. If we ever say something like
"Please do something to get repack to work on user catalog tables,"
that would be a hint, but here we are giving more details about the
error itself.

"User catalog tables" looks good to me too, although the existing
error messages around RelationIsUsedAsCatalogTable() use "table used
as a catalog table," with "system catalogs" meaning the core-provided
ones.

The proposed wording also looks consistent with
errdetail_relkind_not_supported(). So, I used that in the attached
0001.

That said, I also think we can keep the messaging style consistent
within this function. So, in the attached 0002, I changed the existing
error messages for system catalogs, TOAST tables, non-permanent tables
etc. to use detail messages instead of hints, with consistent wording.

Please find the attached v5 patches.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

On Tue, Sep 8, 2026 at 5:47 AM Alvaro Herrera <alvherre@kurilemu.de> wrote:

Hello,

On 2026-Sep-04, Masahiko Sawada wrote:

I reviewed the patch. It's simple and I agree to add a check for user
catalog tables. I've slightly rephrased the comment in
check_concurrent_repack_requirements() to clarify the reason why
REPACK (CONCURRENTLY) doesn't support user catalog tables while it
does for normal tables even though it's a MVCC-unsafe operation. Also
updated the commit message as well. Please review it.

Thanks, I gave this a look and I think it's okay. I'm not sure about
the exact wording of the error message though,

+             ereport(ERROR,
+                             errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                             errmsg("cannot execute %s on relation \"%s\"",
+                                        "REPACK (CONCURRENTLY)", RelationGetRelationName(rel)),
+                             errhint("%s is not supported for tables used as catalog tables.",
+                                             "REPACK (CONCURRENTLY)"));

ERROR: cannot execute REPACK (CONCURRENTLY) on relation "foobar"
HINT: REPACK (CONCURRENTLY) is not supported for tables used as catalog tables.

I think the HINT should really be DETAIL, and I'm not sure we need to
mention the command again there. On the other hand, I'm not really
happy with the term "tables used as catalog tables". In the
documentation we use the term "user provided catalog tables" or simply
"user catalog tables" (see <sect2 id="logicaldecoding-capabilities">).
So how about something like this?

ERROR: cannot execute REPACK (CONCURRENTLY) on relation "foobar"
DETAIL: This operation is not supported for user catalog tables.

Looks good to me.

Would you push this patch, or do you want me to?

I can take it if you are okay, as it's a trivial fix.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

On 2026-Sep-08, Masahiko Sawada wrote:

On Tue, Sep 8, 2026 at 5:47 AM Alvaro Herrera <alvherre@kurilemu.de> wrote:

Would you push this patch, or do you want me to?

I can take it if you are okay, as it's a trivial fix.

Yeah, sounds good, thanks.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"No nos atrevemos a muchas cosas porque son difíciles,
pero son difíciles porque no nos atrevemos a hacerlas" (Séneca)

On Tue, Sep 8, 2026 at 11:57 AM Alvaro Herrera <alvherre@kurilemu.de> wrote:

On 2026-Sep-08, Masahiko Sawada wrote:

On Tue, Sep 8, 2026 at 5:47 AM Alvaro Herrera <alvherre@kurilemu.de> wrote:

Would you push this patch, or do you want me to?

I can take it if you are okay, as it's a trivial fix.

Yeah, sounds good, thanks.

Pushed.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Hi,

On Tue, Sep 8, 2026 at 1:09 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Pushed.

Thanks for taking care of this. Please find another remaining patch
rebased to use detail instead of hints and reword the messages
consistently in REPACK (CONCURRENTLY) checks.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

On Wed, Sep 9, 2026 at 10:28 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

Hi,

On Tue, Sep 8, 2026 at 1:09 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Pushed.

Thanks for taking care of this. Please find another remaining patch
rebased to use detail instead of hints and reword the messages
consistently in REPACK (CONCURRENTLY) checks.

Thank you for rebasing the patch! Pushed.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com