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

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.

--
Best Wishes,
Ashutosh Bapat