Re: More parallel pg_dump bogosities

Tom,

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> >> And lastly, the ROW SECURITY items are ready because they are not marked
> >> with any dependency at all, none, nada. This seems bad. In principle
> >> it could mean that parallel restore would try to emit "ALTER TABLE ENABLE
> >> ROW LEVEL SECURITY" before it's created the table :-(. I think that in
> >> practice that can't happen today, because CREATE TABLE commands get
> >> emitted before we've switched into parallel restore mode at all. But it's
> >> definitely possible that ENABLE ROW LEVEL SECURITY could be emitted before
> >> we've restored the table's data. Won't that break things?
>
> > We certainly wouldn't want the ROW SECURITY items to be emitted before
> > the table itself, that wouldn't work. Emitting ENABLE RLS before
> > restoring the table's data shouldn't actually break anything though as
> > the owner of the table (which is who we're restoring the data as,
> > at that point, right?) will bypass RLS.
>
> Hmm. We'd typically be running a restore either as superuser or as the
> table owner ...

Right.

> > Now, if what's actually set is
> > FORCE RLS, then we may have an issue if that is emitted before the table
> > data since that would cause RLS to be in effect even if we're the owner
> > of the table.
>
> ... but if FORCE RLS is applied to the table, we gotta problem anyway,
> because that command is included right with the initial table creation.
> Once the ENABLE comes out, inserts will fail, because there are no
> policies yet (those *do* have table dependencies that get moved to
> point at the TABLE DATA).

Yeah, I don't think the pg_dump aspect was considered very carefully
when we reworked how the 'force' RLS option works.

> So it's a bug all right, but a sufficiently corner-case one that it's
> not too surprising it hasn't been reported. Even though the ENABLE RLS
> item is "ready to restore", it'll still be at the end of the work list,
> so you'd need a very high parallel worker count to have much chance of
> it coming out before the table's data item gets processed.

Ok.

> >> I think this is easy enough to fix, just force a dependency on the table
> >> to be attached to a ROW SECURITY item; but I wanted to confirm my
> >> conclusion that we need one.
>
> > I do think we should have one.
>
> I'll do that, but ...

Ok.

> > In an ideal world, I think we'd want:
>
> > CREATE TABLE
> > TABLE DATA
> > ENABLE RLS
> > ADD RLS POLICIES
> > GRANT RIGHTS
>
> ... as things stand, even with this fix, a parallel restore will emit
> CREATE POLICY and ENABLE RLS commands in an unpredictable order. Not sure
> how much it matters, though. The GRANTs should come out last anyway.

Yeah, as long as GRANT's come out last, it really shouldn't matter when
the ENABLE happens vs. the CREATE POLICY's, except in the odd FORCE
case.

Thanks!

Stephen