On Tue, Sep 8, 2026 at 5:47 AM Alvaro Herrera <[email protected]> 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


Reply via email to