On Thu, Sep 3, 2026 at 3:44 AM Andres Freund <[email protected]> wrote: > > Hi, > > On 2026-09-02 17:33:38 -0400, Andres Freund wrote: > > - 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. > > An particularly odd thing about that is that some things in > rewriteGraphTable.c actually use a more heavyweight lock level - > is_property_associated_with_label() and get_element_property_expr() use > RowShareLock. I don't really understand why? There's no comment about that > either? > > There are also unexplained uses of RowShareLock in propgraphcmds.c - why is > e.g. pg_attribute opened with RowShareLock? >
+1. All of these require AccessShareLock. I don't see why do they need RowShareLock. An only possible explanation is that the functions were used for DDL at some point but then that usage was replaced by pure reads. While investigating this I also found that a lock on the element table is needed before scanning pg_attribute. Some callers of that function held the lock and some didn't. Will include this in the locking related email. > > Separately (noticed when searching for RowShareLock uses): Isn't it somewhat > of a problem to do a seqscan of PropgraphLabelPropertyRelationId for > /* Remove any orphaned pg_propgraph_property entries */ > > I don't know why that code uses RowShareLock either. Not using AccessShareLock > makes sense, afaict the performDeletion() of PropgraphPropertyRelationId done > during the scan will recurse to PropgraphLabelPropertyRelationId - avoiding > deadlock hazards due to lock upgrades could make sense. But a deletion takes > RowExclusive, not RowShare? So what's the point of RowShare? This shouldn't be required if we implement the SQL/PGQ standard dependency model as is. Will propose in a separate email. -- Best Wishes, Ashutosh Bapat
