Обсуждение: [PATCH] Support reading large objects with pg_read_all_data

Hi Hackers,

It was reported in [1] that pg_dump for a user with pg_read_all_data
fails as pg_read_all_data doesn't have the permission to read large
objects. The discussion on the same thread suggested that this was an
oversight as the goal of pg_read_all_data was to allow running pg_dump
[2].

This patch proposes to fill that gap by modifying
pg_largeobject_aclmask_snapshot to provide ACL_SELECT for the role
PG_READ_ALL_DATA. Note that the patch doesn't make an equivalent
change for PG_WRITE_ALL_DATA as it would effectively provide
pg_write_all_data write access to a system catalog which is explicitly
avoided for system catalogs

Please take a look and let me know what you folks think. If this
approach makes sense, I will update the corresponding docs in the
patch.

Thanks & Regards,
Nitin Motiani
Google


[1] https://www.postgresql.org/message-id/19379-089536632927293f%40postgresql.org
[2] https://www.postgresql.org/message-id/r5a3aqlrrqen2snktdmx5tjeoakp3hmbektlqmeqhij3fqqez4%40zmx3bdscipny

On Thu, Feb 05, 2026 at 03:26:02PM +0530, Nitin Motiani wrote:
> Please take a look and let me know what you folks think. If this
> approach makes sense, I will update the corresponding docs in the
> patch.

At a glance, it looks generally reasonable to me.  In addition to updating
the documentation, I'd recommend adding tests.

-- 
nathan

On Thu, Feb 5, 2026 at 9:14 PM Nathan Bossart <nathandbossart@gmail.com> 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.

Regards,
Nitin Motiani
Google

On Mon, Feb 9, 2026 at 5:25 PM Nitin Motiani <nitinmotiani@google.com> wrote:
>
> On Thu, Feb 5, 2026 at 9:14 PM Nathan Bossart <nathandbossart@gmail.com> 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

On Tue, Feb 10, 2026 at 2:17 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> Hi Nitin, Your patch looks good to me except for some minor
> suggestions/questions.
>

Thanks Dilip for the feedback.

> 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
>

I updated the commit message according to this suggestion.


> 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?
>

I have removed the redundant tests in the latest patch. The original
rationale was to test these functions with different arguments and
empty objects. But on reflection those are unrelated to the acl check.
So I'm only keeping one test per function.

Regards,
Nitin Motiani
Google

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

Thanks Nathan for the feedback.

On Fri, Feb 13, 2026 at 2:51 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> Thanks for the new patch.  Please create a commitfest entry so that this
> patch 1) isn't forgotten and 2) gets tested by cfbot.
>

I have created the commitfest entry
https://commitfest.postgresql.org/patch/6485/.

> 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.
>

Changed the formatting in the doc.

>
> I'd suggest simplifying this a bit by borrowing some lines from the
> surrounding tests.

Changed the test based on your suggestions.

Attaching the latest patch.

Regards,
Nitin Motiani
Google

On Fri, Feb 13, 2026 at 05:35:14PM +0530, Nitin Motiani wrote:
> Attaching the latest patch.

This looks pretty good to me.  I'd like to let it sit on the lists a little
while longer in case anyone else has feedback or objections.  Assuming
those don't materialize in the next week or so, I will proceed with
committing it.

-- 
nathan

On Fri, Feb 13, 2026 at 11:17:28AM -0600, Nathan Bossart wrote:
> This looks pretty good to me.  I'd like to let it sit on the lists a little
> while longer in case anyone else has feedback or objections.  Assuming
> those don't materialize in the next week or so, I will proceed with
> committing it.

Here's what I have staged for commit.  I didn't understand the reasoning
behind not giving pg_write_all_data privileges on large objects.  Your
commit message mentions that "granting write access would imply write
permissions on a system catalog" (which I assume is referring to
pg_largeobject), but if granting UPDATE on a large object is sufficient to
allow updating portions of that catalog, then I see no reason to be so
strict with pg_write_all_data.  It still doesn't allow updating the catalog
directly.

-- 
nathan

Committed.

-- 
nathan

> Here's what I have staged for commit.  I didn't understand the reasoning
> behind not giving pg_write_all_data privileges on large objects.

Thanks Nathan. My thinking behind this was that even without these
changes, the 'select *' on the large object table worked for
pg_read_all_data so providing access to functions like lo_get seemed
consistent with that behaviour. But for pg_write_all_data, that wasn't
the case so I thought it might be safer not to provide access.

> commit message mentions that "granting write access would imply write
> permissions on a system catalog" (which I assume is referring to
> pg_largeobject), but if granting UPDATE on a large object is sufficient to
> allow updating portions of that catalog, then I see no reason to be so
> strict with pg_write_all_data.  It still doesn't allow updating the catalog
> directly.
>

Thanks for the explanation and taking care of this.

Regards,
Nitin Motiani
Google