On Wed, Aug 26, 2026 at 7:32 AM Kyotaro Horiguchi <[email protected]> wrote: > > Both changes in the current patch look good to me. > > At Tue, 25 Aug 2026 19:11:27 +0530, Amit Kapila <[email protected]> > 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.
