On Thu, Jun 16, 2022 at 03:42:06PM +0900, Yugo NAGATA wrote:
> I am not sure if we should use PreventCommandIfReadOnly in lo_write()
> because there are other public functions that write to catalogs but there
> are not the similar restrictions in such functions. I think it is caller's
> responsibility to prevent to use such public functions in read-only context.
I'd be really tempted to remove the plug on this one, actually.
However, that would also mean to break something just for the sake of
breaking it. So perhaps you are right at the end in that it is better
to let this code be, without the new check.
> I also fixed to raise the error in each of lo_truncate and lo_truncate64
> per Michael's comment in the other post.
Thanks! That counts for 10 SQL functions blocked with 10 tests. So
you have all of them covered.
Looking at the docs of large objects, as of "Client Interfaces", we
mention that any action must take place in a transaction block.
Shouldn't we add a note that no write operations are allowed in a
read-only transaction?
+ if (mode & INV_WRITE)
+ PreventCommandIfReadOnly("lo_open() in write mode");
Nit. This breaks translation. I think that it could be switched to
"lo_open(INV_WRITE)" instead as the flag name is documented.
The patch is forgetting a refresh for largeobject_1.out.
--- INV_READ = 0x20000
--- INV_WRITE = 0x40000
+-- INV_READ = 0x40000
+-- INV_WRITE = 0x20000
Good catch! This one is kind of independent, so I have fixed it
separately, in all the expected output files.
--
Michael