Thread

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

Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-05-18T00:22:19Z

On Sun, May 17, 2026 at 5:12 PM Junwang Zhao <zhjwpku@gmail.com> wrote:
>
>
>
>
> Regards
> Junwang Zhao
>
> On Mon, May 18, 2026 at 07:29 Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:
>>
>> On Fri, May 15, 2026 at 8:43 PM Ayush Tiwari
>> <ayushtiwari.slg01@gmail.com> wrote:
>> >
>> > Hi,
>> >
>> >
>> > On Fri, 15 May 2026 at 20:31, Junwang Zhao <zhjwpku@gmail.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@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@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.

Thanks for noticing. Here's attached correct patch.

-- 
Best Wishes,
Ashutosh Bapat