On Wed, 26 Aug 2026 at 07:32, Kyotaro Horiguchi <[email protected]> wrote: > > Hello, > > 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. > > 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
