On Sat, 28 May 2022 18:00:54 +0900
Michael Paquier <[email protected]> wrote:
> On Fri, May 27, 2022 at 03:30:28PM +0900, Yugo NAGATA wrote:
> > Currently, lo_creat(e), lo_import, lo_unlink, lowrite, lo_put,
> > and lo_from_bytea are allowed even in read-only transactions.
> > By using them, pg_largeobject and pg_largeobject_metatable can
> > be modified in read-only transactions and the effect remains
> > after the transaction finished. Is it unacceptable behaviours,
> > isn't it?
>
> Well, there is an actual risk to break applications that have worked
> until now for a behavior that has existed for years with zero
> complaints on the matter, so I would leave that alone. Saying that, I
> don't really disagree with improving the error messages a bit if we
> are in recovery.
Thank you for your comment. I am fine with leaving the behaviour in
read-only transactions as is if anyone don't complain and there are no
risks.
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.
Regards,
Yugo Nagata
--
Yugo NAGATA <[email protected]>
diff --git a/src/backend/libpq/be-fsstubs.c b/src/backend/libpq/be-fsstubs.c
index 5804532881..55b25b19ea 100644
--- a/src/backend/libpq/be-fsstubs.c
+++ b/src/backend/libpq/be-fsstubs.c
@@ -245,6 +245,8 @@ be_lo_creat(PG_FUNCTION_ARGS)
{
Oid lobjId;
+ PreventCommandDuringRecovery("lo_creat()");
+
lo_cleanup_needed = true;
lobjId = inv_create(InvalidOid);
@@ -256,6 +258,8 @@ be_lo_create(PG_FUNCTION_ARGS)
{
Oid lobjId = PG_GETARG_OID(0);
+ PreventCommandDuringRecovery("lo_create()");
+
lo_cleanup_needed = true;
lobjId = inv_create(lobjId);
@@ -306,6 +310,8 @@ be_lo_unlink(PG_FUNCTION_ARGS)
{
Oid lobjId = PG_GETARG_OID(0);
+ PreventCommandDuringRecovery("lo_unlink()");
+
/*
* Must be owner of the large object. It would be cleaner to check this
* in inv_drop(), but we want to throw the error before not after closing
@@ -368,6 +374,8 @@ be_lowrite(PG_FUNCTION_ARGS)
int bytestowrite;
int totalwritten;
+ PreventCommandDuringRecovery("lowrite()");
+
bytestowrite = VARSIZE_ANY_EXHDR(wbuf);
totalwritten = lo_write(fd, VARDATA_ANY(wbuf), bytestowrite);
PG_RETURN_INT32(totalwritten);
@@ -413,6 +421,8 @@ lo_import_internal(text *filename, Oid lobjOid)
LargeObjectDesc *lobj;
Oid oid;
+ PreventCommandDuringRecovery("lo_import()");
+
/*
* open the file to be read in
*/
@@ -539,6 +549,8 @@ lo_truncate_internal(int32 fd, int64 len)
{
LargeObjectDesc *lobj;
+ PreventCommandDuringRecovery("lo_truncate()");
+
if (fd < 0 || fd >= cookies_size || cookies[fd] == NULL)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
@@ -815,6 +827,8 @@ be_lo_from_bytea(PG_FUNCTION_ARGS)
LargeObjectDesc *loDesc;
int written PG_USED_FOR_ASSERTS_ONLY;
+ PreventCommandDuringRecovery("lo_from_bytea()");
+
lo_cleanup_needed = true;
loOid = inv_create(loOid);
loDesc = inv_open(loOid, INV_WRITE, CurrentMemoryContext);
@@ -837,6 +851,8 @@ be_lo_put(PG_FUNCTION_ARGS)
LargeObjectDesc *loDesc;
int written PG_USED_FOR_ASSERTS_ONLY;
+ PreventCommandDuringRecovery("lo_put()");
+
lo_cleanup_needed = true;
loDesc = inv_open(loOid, INV_WRITE, CurrentMemoryContext);
