On Thu, Jul 2, 2026 at 9:10 AM zengman <[email protected]> wrote: > > > The word properties in the commit message means different things in > > different contexts leading to a possible confusion. Commit message > > below is clearer, I think > > > > AlterPropGraph() cleans up pg_propgraph_property entries that are > > orphaned by dropping an element or by dropping properties associated > > with an element. But it doesn't clean up pg_propgraph_property entries > > that are orphaned by dropping labels associated with an element. Fix this > > missing case. > > > > Also the comment in the test may read better if rewritten like below > > > > -- Dropping a label should drop only orphaned properties. Dropping label > > t3l1 > > -- should also drop zz because it is only associated with label t3l2. But x > > is > > -- not dropped, even if it is associated with t3l2, because it remains > > -- associated with t3l1. zz will not appear in the information schema > > queries > > -- outputs below, but x will. > > > > I did not change the comment to mention pg_propgraph_properties since > > the comment is clear even without mentioning it. > > > > What do you think? > > > > I have verified that the patch you have attached applies cleanly on > > master. But it may not apply cleanly in case another change to > > create_property_graph.sql goes in first. We will provide rebased patch > > if that happens. > > Hi Ashutosh, > > Thanks for the review. I've updated the commit message per your suggestion — > your version is clearer, so I've adopted it as-is. >
Thanks. > Regarding the test comment: I kept the original wording. The current comment > already describes the case accurately (zz is orphaned because it's only > associated with the dropped > label t3l2, while x is not orphaned because it remains associated with t3l1), > so I left it unchanged. > Let's defer this to committer's judgement. -- Best Wishes, Ashutosh Bapat
