On Mon, May 30, 2022 at 05:44:18PM +0900, Yugo NAGATA wrote: > As to the error messages during recovery, I think it is better to improve > it, because the current messages are emitted by elog() and it seems to imply > they are internal errors that we don't expected. I attached a updated patch > for it.
Yeah, elog() messages should never be user-facing as they refer to internal errors, and any of those errors are rather deep in the tree while being unexpected. lo_write() is published in be-fsstubs.h, though we have no callers of it in the backend for the core code. Couldn't there be a point in having the recovery protection there rather than in the upper SQL routine be_lowrite()? At the end, we would likely generate a failure when attempting to insert the LO data in the catalogs through inv_api.c, but I was wondering if we should make an extra effort in improving the report also in this case if there is a direct caller of this LO write routine. The final picture may be better if we make lo_write() a routine static to be-fsstubs.c but it is available for ages, so I'd rather leave it declared as-is. libpq fetches the OIDs of the large object functions and caches it for PQfn() as far as I can see, so it is fine by me to have the protections in be-fsstubs.c, letting inv_api.c deal with the internals with the catalogs, ACL checks, etc. Should we complain on lo_open() with the write mode though? The change for lo_truncate_internal() is a bit confusing for the 64b version, as we would complain about lo_truncate() and not lo_truncate64(). While on it, could we remove -DFSDB? -- Michael
signature.asc
Description: PGP signature
