Re: PGQ catalog representation and pg_dump support

Hi Andres, Robert,

I am looking at all the issues reported in this thread to summarize
the things we need take care of in the next round. I will send a
summary at the end and start discussion on topics that need further
discussion. I am replying to individual discussions here.

On Fri, Sep 4, 2026 at 1:51 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> Hi,
>
> On 2026-09-03 15:47:32 -0400, Robert Haas wrote:
> > On Wed, Sep 2, 2026 at 5:33 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > > This isn't just bad when the graph table query is in a view (where the
> > > parser wouldn't have acquired locks), afaict parse-analysis doesn't acquire
> > > any locks on the element tables?
> >
> > I'm having trouble parsing this sentence. Are there some extra
> > negatives in here someplace, or some missing ones?
>
> Something indeed isn't quite right, I think I was mainly just being too terse
> (IIRC I had a longer explanation there that felt redundant, that I shortened
> ... too much).
>
> What I wanted to draw attention to is that there are two different "locking
> schemes" related to rewrite/ code. Rewrites on plain queries can rely on
> locks being acquired during parse-analysis. Queries that originate in views
> (and other rule related stuff) however, can't rely on that, since
> parse-analysis is done during the view's creation. Therefore in the view case,
> we rely on acquiring locks via AcquireRewriteLocks().
>
> Here I think we acquire locks on the graph table elements in neither of the
> two paths.
>

Once the proper dependencies (after they are fixed) are in place , the
metadata of the element tables needed for rewriting the GRAPH_TABLE
can not be altered because of dependencies. E.g. changing column type
results in an error. I couldn't reproduce any hazards in this area.
However, I agree that we should take locks on the element tables
before accessing their metadata.

>
> > > - As mentioned in my last email, nothing protects the graph object against
> > > being altered while rewriteGraphTable() is running, due to AlterPropGraph()
> > > only holding an AccessShareLock. That's bad.
> >
> > AlterPropGraph() takes ShareRowExclusiveLock on the property graph
> > itself. The comment justifies why the lock level isn't weaker, but not
> > why it isn't stronger; normally, DDL takes AccessExclusiveLock, and
> > that would be my baseline expectation here, too.
>
> Yep. Seems pretty clear that AlterPropGraph() needs an AEL as it can affect
> the "result shape". It's only ok to use a lower lock level if it's ok to
> continue operating with an outdated definition of the object.
>
> I guess we could make uses of the propgraph use a more heavyweight lock
> instead, but ... :)
>

Agreed. AlterPropGraph needs AccessExclusiveLock.

>
> > > - Also bad: Afaict a table or column - involved in a property graph - being
> > > dropped, does not seem to actually lock the property graph? Which means that
> > > could happen concurrently with rewriteGraphTable() even if AlterPropGraph()
> >
> > I think the end of this sentence got cut off.
>
> Ooops. I guess I was intending to say something like "even if
> AlterPropGraph() were to take an AEL, as a table that is part of the property
> graph could be dropped without going through AlterPropGraph(), and therefore
> without locking the propgraph with an AEL".
>

Yes, this is again missing from the depedency handling. I think when
considering a property graph component in dependency list
construction, we have to take an AEL on the property graph. I will
include this point in my dependency thread.

I will send a separate email about locking in property graph usage
consolidating all the issues in one email.

--
Best Wishes,
Ashutosh Bapat