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