Re: PGQ catalog representation and pg_dump support

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.

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

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

> [ more problems]
>
> To me, this class of problem seems completely unacceptable in a
> committed feature.

A small number of oversights of this class, in a feature of the size of PGQ,
doesn't necessarily seem entirely unacceptable to me. We are all human, and we
don't always have an existing pattern to model things after, making it harder
to get it right (e.g. I don't think we have a real precedent for the type of
dependencies that are needed here).

But the density of such problems in PGQ, at this stage of the release cycle
(and, TBH, probably at commit time), doesn't seem OK.

Greetings,

Andres Freund