Re: Prevent writes on large objects in read-only transactions

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