On Mon, Feb 9, 2026 at 5:25 PM Nitin Motiani <[email protected]> wrote:
>
> On Thu, Feb 5, 2026 at 9:14 PM Nathan Bossart <[email protected]> 
> wrote:
> > At a glance, it looks generally reasonable to me.  In addition to updating
> > the documentation, I'd recommend adding tests.
> >
>
> Thanks Nathan. I'm attaching the patch with new tests and updated
> documentation. Please take a look.

Hi Nitin, Your patch looks good to me except for some minor
suggestions/questions.

1. I think we can change the commit message slightly, and also removed
the part which says added doc/test
Suggestion:
Support large object functions with pg_read_all_data

Allow members of the pg_read_all_data predefined role to access large
objects via the large object functional API (e.g., lo_get, loread).

Previously, while pg_read_all_data permitted direct SELECT queries
on the pg_largeobject catalog table, it did not grant the necessary
permissions to use the logical large object functions. This created an
inconsistency in how the role accessed data stored in large objects
versus standard relations.

This change updates the ACL check for large objects to recognize
pg_read_all_data as having SELECT privileges. Note that this
support is intentionally omitted for pg_write_all_data, as granting
write access would imply write permissions on a system catalog, a
privilege level that pg_write_all_data does not currently provide
for other objects.

2. +SELECT lo_get(1002); -- ok
+          lo_get
+--------------------------
+ \x68656c6c6f20776f726c64
+(1 row)
+
+SELECT lo_get(1002, 6, 5); -- ok
+    lo_get
+--------------
+ \x776f726c64
+(1 row)

Isn't it sufficient to just have second lo_get test i.e. SELECT
lo_get(1002, 6, 5);, is there anything extra we are checking with the
first test or is it just testing the same?

Check other tests as well for loread(), seems there are multiple
loread() tests that are testing the same functionality?

--
Regards,
Dilip Kumar
Google


Reply via email to