On Mon, Jun 29, 2026 at 8:23 AM zengman <[email protected]> wrote:
>
> > It does conflict with a fix in my branch for issues in [1]. Since
> > Zengman is the original author of the patch, I was expecting him to
> > take the minimal reproduction suggested in my patch and provide an
> > updated patch. But it looks like he didn't get time to do that. I
> > suggest that we tackle the patches in [1] first and then tackle this
> > issue.
>
> Hi,
>
> Apologies for the delay -- I've been caught up with some other commitments. 
> Thanks, Ashutosh, for the review and the improved tests.
> The fix doesn't depend on the other patches; it applies cleanly on current 
> master. Rebased patch attached, incorporating the review comments.

Thanks.

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.

-- 
Best Wishes,
Ashutosh Bapat


Reply via email to