Re: (SQL/PGQ) cache lookup failed for label

On Mon, May 18, 2026 at 8:22 AM Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
> On Sun, May 17, 2026 at 5:12 PM Junwang Zhao <zhjwpku(at)gmail(dot)com> wrote:
> >
> >
> >
> >
> > Regards
> > Junwang Zhao
> >
> > On Mon, May 18, 2026 at 07:29 Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> >>
> >> On Fri, May 15, 2026 at 8:43 PM Ayush Tiwari
> >> <ayushtiwari(dot)slg01(at)gmail(dot)com> wrote:
> >> >
> >> > Hi,
> >> >
> >> >
> >> > On Fri, 15 May 2026 at 20:31, Junwang Zhao <zhjwpku(at)gmail(dot)com> wrote:
> >> >>
> >> >> Hi Ayush,
> >> >>
> >> >> >>
> >> >> >> I also added regression coverage for both cases:
> >> >> >>
> >> >> >> DROP LABEL of a label used by a GRAPH_TABLE view
> >> >> >> DROP PROPERTIES of a property used by a GRAPH_TABLE view
> >> >> >>
> >> >> >> Both now fail with the normal dependency error until the view is dropped.
> >> >> >>
> >> >> >> Thoughts?
> >> >>
> >> >> I'd suggest adding two stmts to the regression that can cover that walk of
> >> >> graph_table_columns is also working.
> >> >>
> >> >> [local] zhjwpku(at)postgres:5432-52789=# ALTER PROPERTY GRAPH myshop
> >> >> ALTER VERTEX TABLE customers ALTER LABEL customers DROP PROPERTIES
> >> >> (name);
> >> >> ALTER PROPERTY GRAPH
> >> >> Time: 1.312 ms
> >> >>
> >> >> [local] zhjwpku(at)postgres:5432-52789=# ALTER PROPERTY GRAPH myshop
> >> >> ALTER VERTEX TABLE products ALTER LABEL products DROP PROPERTIES
> >> >> (name);
> >> >> ERROR: cannot drop property name of property graph myshop because
> >> >> other objects depend on it
> >> >> DETAIL: view customers_us depends on property name of property graph myshop
> >> >> HINT: Use DROP ... CASCADE to drop the dependent objects too.
> >> >> Time: 2.231 ms
> >> >>
> >> >
> >> > Good point, thanks. I added that coverage in the attached v3.
> >> >
> >> > The test now also drops customers.name first, which is allowed because the
> >> > graph-level property still exists via products.name, and then verifies that
> >> > dropping products.name is rejected with the dependency error from
> >> > customers_us. That should cover GraphPropertyRef nodes reached through the
> >> > GRAPH_TABLE COLUMNS list, in addition to the existing label and graph-pattern
> >> > property cases.
> >> >
> >> > I re-added customers.name afterward so the existing myshop graph remains
> >> > unchanged for the following tests.
> >>
> >> Thanks Ayush for working on this and providing the patch. Thanks
> >> Junwang for reviewing it.
> >>
> >> I have some more comments.
> >>
> >> }
> >> + else if (IsA(node, GraphLabelRef))
> >> + {
> >> + GraphLabelRef *glr = (GraphLabelRef *) node;
> >> +
> >> + /*
> >> + * GRAPH_TABLE label reference: depend on the label catalog entry.
> >> + * No expression substructure to recurse into.
> >>
> >> That comment is correct, however, the case doesn't return false,
> >> giving an impression that we are recursing into the substructure.
> >> expression_tree_walker() then returns false. But I am seeing an
> >> inconsistency in when to "return false" and when not to. For example,
> >> for some primitive nodes in expression_tree_walker() like Var, this
> >> function returns false. But for other primitive nodes like Param it
> >> doesn't. And there's not comment explaining this difference. I guess
> >> newer additions to this function are relying on expression_tree_walker
> >> to return false. So I just removed the misleading comment and let the
> >> two new nodes rely on expression_tree_walker().
> >>
> >> + */
> >> + add_object_address(PropgraphLabelRelationId, glr->labelid, 0,
> >> + context->addrs);
> >> + }
> >> + else if (IsA(node, GraphPropertyRef))
> >> + {
> >> + GraphPropertyRef *gpr = (GraphPropertyRef *) node;
> >> +
> >> + /* GRAPH_TABLE property reference: depend on the property entry. */
> >> + add_object_address(PropgraphPropertyRelationId, gpr->propid, 0,
> >> + context->addrs);
> >> + }
> >>
> >> @@ -536,6 +536,22 @@ SELECT g.* FROM x1,
> >> ORDER BY customer_name, product_name;
> >> SELECT pg_get_viewdef('customers_us'::regclass);
> >> +-- A view defined over GRAPH_TABLE should record dependencies on the labels
> >> +-- and properties it references, so they cannot be dropped from under it.
> >> +ALTER PROPERTY GRAPH myshop ALTER EDGE TABLE order_items DROP LABEL list_items;
> >> +ALTER PROPERTY GRAPH myshop ALTER EDGE TABLE wishlist_items
> >> + DROP LABEL list_items; -- error
> >> +ALTER PROPERTY GRAPH myshop ALTER EDGE TABLE order_items
> >> + ADD LABEL list_items PROPERTIES (order_id AS link_id, product_no);
> >> +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers
> >> + ALTER LABEL customers DROP PROPERTIES (address); -- error
> >> +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers
> >> + ALTER LABEL customers DROP PROPERTIES (name);
> >> +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products
> >> + ALTER LABEL products DROP PROPERTIES (name); -- error
> >> +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers
> >> + ALTER LABEL customers ADD PROPERTIES (name);
> >> +
> >>
> >> Without the code changes, we do not see "cache lookup failed for label
> >> " error, because there is nothing that uses this view after the drop.
> >> In the attached patch, I have moved the DDL statements before
> >> pg_get_viewdef() which throws "cache lookup failed" error without code
> >> changes and for every DDL statement when we try it separately.
> >>
> >> My earlier comment or the test by Man might have misled you into
> >> thinking that we need to drop properties or labels which are defined
> >> multiple times so that we test that the dependency error does not
> >> trigger when a property or a label is not orphaned. Sorry if that's
> >> the case. I don't think that's the goal here. Further, such tests
> >> require additional DDL to restore property graph state and also change
> >> the view definition produced by pg_get_viewdef(). So I used DDLs that
> >> drop properties or labels which are defined only once.

+1 for this change, we don't need to re-add the label and property to myshop.

> >>
> >> I shortened the commit message by taking essential elements from your
> >> commit message.
> >>
> >> Please review the attached patch.
> >
> >
> > The attached patch seems not for this thread.
>
> Thanks for noticing. Here's attached correct patch.

The patch LGTM, thanks for taking care of this.

>
> --
> Best Wishes,
> Ashutosh Bapat

--
Regards
Junwang Zhao