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

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

>
>
> --
> Best Wishes,
> Ashutosh Bapat
>