Thread

Commits

  1. Allow pg_{read,write}_all_data to access large objects.

  1. [PATCH] Support reading large objects with pg_read_all_data

    Nitin Motiani <nitinmotiani@google.com> — 2026-02-05T09:56:02Z

    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
    
  2. Re: [PATCH] Support reading large objects with pg_read_all_data

    Nathan Bossart <nathandbossart@gmail.com> — 2026-02-05T15:44:50Z

    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
    
    
    
    
  3. Re: [PATCH] Support reading large objects with pg_read_all_data

    Nitin Motiani <nitinmotiani@google.com> — 2026-02-09T11:55:19Z

    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
    
  4. Re: [PATCH] Support reading large objects with pg_read_all_data

    Dilip Kumar <dilipbalaut@gmail.com> — 2026-02-10T08:47:14Z

    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
    
    
    
    
  5. Re: [PATCH] Support reading large objects with pg_read_all_data

    Nitin Motiani <nitinmotiani@google.com> — 2026-02-10T13:09:18Z

    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
    
  6. Re: [PATCH] Support reading large objects with pg_read_all_data

    Nathan Bossart <nathandbossart@gmail.com> — 2026-02-12T21:21:54Z

    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
    
    
    
    
  7. Re: [PATCH] Support reading large objects with pg_read_all_data

    Nitin Motiani <nitinmotiani@google.com> — 2026-02-13T12:05:14Z

    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
    
  8. Re: [PATCH] Support reading large objects with pg_read_all_data

    Nathan Bossart <nathandbossart@gmail.com> — 2026-02-13T17:17:28Z

    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
    
    
    
    
  9. Re: [PATCH] Support reading large objects with pg_read_all_data

    Nathan Bossart <nathandbossart@gmail.com> — 2026-02-23T17:07:21Z

    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
    
  10. Re: [PATCH] Support reading large objects with pg_read_all_data

    Nathan Bossart <nathandbossart@gmail.com> — 2026-02-23T21:00:20Z

    Committed.
    
    -- 
    nathan
    
    
    
    
  11. Re: [PATCH] Support reading large objects with pg_read_all_data

    Nitin Motiani <nitinmotiani@google.com> — 2026-02-24T02:06:45Z

    > 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