On Tue, Feb 10, 2026 at 06:39:18PM +0530, Nitin Motiani wrote:
> I updated the commit message according to this suggestion.

Thanks for the new patch.  Please create a commitfest entry so that this
patch 1) isn't forgotten and 2) gets tested by cfbot.

        <literal>pg_read_all_data</literal> allows reading all data (tables,
-       views, sequences), as if having <command>SELECT</command> rights on
-       those objects and <literal>USAGE</literal> rights on all schemas.  This
+       views, sequences, and large objects), as if having
+       <command>SELECT</command> rights on those objects and
+       <literal>USAGE</literal> rights on all schemas.  This

nitpick: I usually try to make small doc updates in a way that doesn't
disturb the surrounding lines so that we retain as much git history as
possible.  For example, in this case, I would've probably done something
like:

        <literal>pg_read_all_data</literal> allows reading all data (tables,
-       views, sequences), as if having <command>SELECT</command> rights on
+       views, sequences, large objects), as if having 
<command>SELECT</command> rights on
        those objects and <literal>USAGE</literal> rights on all schemas.  This

Of course, this isn't always possible, and opinions may vary...

+\c -
+-- confirm pg_read_all_data implies read access to large objects
+SELECT lowrite(lo_open(1002, x'20000'::int), 'hello world');
+
+SET SESSION AUTHORIZATION regress_priv_user6;
+SELECT has_largeobject_privilege(1002, 'SELECT'); -- true
+SELECT has_largeobject_privilege(1002, 'UPDATE'); -- false
+SELECT lo_get(1002); -- ok
+SELECT lowrite(lo_open(1002, x'20000'::int), 'abcd'); -- fail
+do $$
+declare
+  fd int;
+begin
+  fd := lo_open(1002, x'40000'::int);
+  perform lo_lseek(fd, 6, 0);
+  raise notice 'position after lseek: %', lo_tell(fd);
+  raise notice 'data read: %', loread(fd, 5);
+  raise notice 'position after loread: %', lo_tell(fd);
+  perform lo_close(fd);
+end;
+$$;
+
+\c -
+SELECT lo_truncate(lo_open(1002, x'20000'::int), 0);    -- ok

I'd suggest simplifying this a bit by borrowing some lines from the
surrounding tests.  I don't think we need to test lo_lseek and friends
because (IIUC) those don't need to do ACL checks.  Here's a rough idea of
what I'm thinking:

+\c -
+-- confirm member of pg_read_all_data can read large objects
+SET SESSION AUTHORIZATION regress_priv_user6;
+
+SELECT loread(lo_open(1002, x'40000'::int), 32);
+SELECT lo_get(1002);
+SELECT lowrite(lo_open(1002, x'20000'::int), 'abcd');   -- to be denied
+SELECT lo_unlink(1002);                                 -- to be denied
+SELECT lo_truncate(lo_open(1002, x'20000'::int), 10);   -- to be denied
+SELECT lo_put(1002, 1, 'abcd');                         -- to be denied

Later on, there's also this change:

-SELECT has_largeobject_privilege('regress_priv_user6', 1008, 'SELECT'); -- no
+SELECT has_largeobject_privilege('regress_priv_user6', 1008, 'SELECT'); -- yes

I think the point of this test is to have a negative case, so we should
probably switch it to another role that doesn't have privileges on the
large object.  But it wouldn't hurt to also verify
has_largeobject_privilege() works as expected for members of
pg_read_all_data.

-- 
nathan


Reply via email to