Re: [PATCH] Support reading large objects with pg_read_all_data

From: Nathan Bossart <nathandbossart(at)gmail(dot)com>
To: Nitin Motiani <nitinmotiani(at)google(dot)com>
Cc: Dilip Kumar <dilipbalaut(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Support reading large objects with pg_read_all_data
Date: 2026-02-12 21:21:54
Message-ID: aY5EcqjJjmeNLG4Y@nathan
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2026-02-12 21:25:49 Re: Speed up COPY TO text/CSV parsing using SIMD
Previous Message Matheus Alcantara 2026-02-12 21:08:57 Re: Add CREATE SCHEMA ... LIKE support