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

On Wed, Aug 26, 2026 at 7:32 AM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> 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.
>

I think the UTF-8 concern would apply only if relkind contained a
value outside the currently defined RELKIND_* values. All the valid
relkind values defined in pg_class.h are ASCII characters (r, i, S,
t,etc.), so %c is safe for the values that relkind is expected to
contain.

That said, I agree that %d would be safer if we want to make the error
handling robust against an arbitrary/invalid byte say due to catalog
corruption. As Vignesh pointed out, though, %c is currently the common
convention for printing relkind, whereas %d is used only in one place.

So perhaps we should keep %c in this patch and, if we want to adopt %d
for unexpected relkind values, let's handle the existing %c usages
consistently in a separate patch.

Thoughts?

--
With Regards,
Amit Kapila.