Re: Assertion failure in GetSubscriptionRelations() with concurrent DROP TABLE

On Wed, 26 Aug 2026 at 07:32, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> Hello,
>
> Both changes in the current patch look good to me.
>
> At Tue, 25 Aug 2026 19:11:27 +0530, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote in
> > > I feel \"%c\" might be better than %d, like what ProcessUtilitySlow() does,
> > > because the relkind is defined as char.
> > >
> >
> > Agreed, though %d is used for the same purpose at one place in code
> > but other uses %c for relkind. How about a slightly more informative
> > message like: elog(ERROR, "unexpected relkind \"%c\" for relation %u
> > in subscription %u", relkind, subrel->srrelid, subid)?
>
> I may be being overly cautious, but I'm a little hesitant to use %c
> here. For example, if the value happened to be the first byte of a
> UTF-8 sequence, it could result in an invalidly encoded log line.
>
> Since this error is unlikely to be seen in practice and readability is
> not particularly important here, I think %d is safer. It might even be
> better to change the existing uses of %c for relkind to %d for the
> same reason.

I checked how relkind is formatted elsewhere in the codebase.
%c is used more commonly like in the following places:
heapam.c — "missing lock for relation \"%s\" (OID %u, relkind %c) @ TID (%u,%u)"
utility.c — "unexpected relkind \"%c\" on partition \"%s\""
postgres_fdw.c — "remote table \"%s.%s\" is of relkind \"%c\" which
cannot have statistics"
pg_dump.c — "cannot dump statistics for relation kind \"%c\""
pg_class.c — "unrecognized relkind: '%c'"

%d is used in only one place:
tablecmds.c — "unexpected relkind: %d"

So %c is clearly the more common convention for relkind. I think we
should follow the existing convention and use %c in this patch. If we
decide that %d is preferable, it would be better to change all of the
%c uses in a separate patch.
Thoughts?

Regards,
Vignesh