On Sun, May 17, 2026 at 5:12 PM Junwang Zhao <[email protected]> wrote: > > > > > Regards > Junwang Zhao > > On Mon, May 18, 2026 at 07:29 Ashutosh Bapat <[email protected]> > wrote: >> >> On Fri, May 15, 2026 at 8:43 PM Ayush Tiwari >> <[email protected]> wrote: >> > >> > Hi, >> > >> > >> > On Fri, 15 May 2026 at 20:31, Junwang Zhao <[email protected]> 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
From e2f8e1562a8e863fc896442175892e6a011a593b Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Sun, 17 May 2026 12:21:13 +0530 Subject: [PATCH v20260517 5/5] Record dependencies on graph labels and properties A view definition with GRAPH_TABLE depends upon the property graph it references as well as the properties and labels referenced in it. We recorded the dependency on the property graph, but did not record dependency on labels and properties. This allowed properties or labels referenced by a view to be dropped resulting in a cache lookup error when such a view was accessed. Fix this bug by handling GraphPropertyRef and GraphLabelRef in find_expr_references_walker(). The dependency on the data type of property is not needed to be recorded separately as it is recorded indirectly via dependency on the property graph property itself. Please note that a property or a label associated with individual elements can still be dropped as long as there are other elements which are associated with that property or label since they do not lead to dropping the property or the label from the property graph altogether. Reported-by: Man Zeng <[email protected]> Author: Ayush Tiwari <[email protected]> Author: Ashutosh Bapat <[email protected]> Reviewed by: Junwang Zhao <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/catalog/dependency.c | 19 ++++++++++++ src/test/regress/expected/graph_table.out | 37 +++++++++++++++++------ src/test/regress/sql/graph_table.sql | 14 +++++++-- 3 files changed, 59 insertions(+), 11 deletions(-) diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index fdb8e67e1f5..c54774b3275 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -2165,6 +2165,25 @@ find_expr_references_walker(Node *node, add_object_address(TypeRelationId, rowexpr->row_typeid, 0, context->addrs); } + else if (IsA(node, GraphLabelRef)) + { + GraphLabelRef *glr = (GraphLabelRef *) node; + + /* GRAPH_TABLE label reference depends on the property graph label */ + add_object_address(PropgraphLabelRelationId, glr->labelid, 0, + context->addrs); + } + else if (IsA(node, GraphPropertyRef)) + { + GraphPropertyRef *gpr = (GraphPropertyRef *) node; + + /* + * GRAPH_TABLE property reference depends on the property graph + * property + */ + add_object_address(PropgraphPropertyRelationId, gpr->propid, 0, + context->addrs); + } else if (IsA(node, RowCompareExpr)) { RowCompareExpr *rcexpr = (RowCompareExpr *) node; diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index e8d49fd5cd4..a5f9f0ac90a 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -929,7 +929,7 @@ SELECT * FROM GRAPH_TABLE (g4 MATCH (s WHERE s.id = 3)-[e]-(d) COLUMNS (s.val, e 30 | 300 | 10 (2 rows) --- ruleutils reverse parsing +-- GRAPH_TABLE in views -- The query in the view definition is intentionally complex to test one view with many -- features like label disjunction, lateral references, WHERE clauses in graph -- patterns. @@ -938,16 +938,35 @@ SELECT g.* FROM x1, GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US' AND c.customer_id = x1.a) -[IS customer_orders | customer_wishlists ]-> (l IS orders | wishlists)-[ IS list_items]->(p IS products) - COLUMNS (c.name AS customer_name, p.name AS product_name, x1.a AS a)) g + COLUMNS (c.name AS customer_name, p.name AS product_name, p.price, x1.a AS a)) g ORDER BY customer_name, product_name; +-- Dropping properties or labels used by a view is not allowed +-- If these DDLs succeed, the pg_get_viewdef call below will throw cache lookup +-- error. +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE orders DROP LABEL orders; -- error +ERROR: cannot drop label orders of property graph myshop because other objects depend on it +DETAIL: view customers_us depends on label orders of property graph myshop +HINT: Use DROP ... CASCADE to drop the dependent objects too. +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers + ALTER LABEL customers DROP PROPERTIES (address); -- error +ERROR: cannot drop property address of property graph myshop because other objects depend on it +DETAIL: view customers_us depends on property address of property graph myshop +HINT: Use DROP ... CASCADE to drop the dependent objects too. +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products + ALTER LABEL products DROP PROPERTIES (price); -- error +ERROR: cannot drop property price of property graph myshop because other objects depend on it +DETAIL: view customers_us depends on property price of property graph myshop +HINT: Use DROP ... CASCADE to drop the dependent objects too. +-- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); - pg_get_viewdef ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- - SELECT g.customer_name, + - g.product_name, + - g.a + - FROM x1, + - GRAPH_TABLE (myshop MATCH (c IS customers WHERE (((c.address)::text = 'US'::text) AND (c.customer_id = x1.a)))-[IS customer_orders|customer_wishlists]->(l IS orders|wishlists)-[IS list_items]->(p IS products) COLUMNS (c.name AS customer_name, p.name AS product_name, x1.a AS a)) g+ + pg_get_viewdef +---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + SELECT g.customer_name, + + g.product_name, + + g.price, + + g.a + + FROM x1, + + GRAPH_TABLE (myshop MATCH (c IS customers WHERE (((c.address)::text = 'US'::text) AND (c.customer_id = x1.a)))-[IS customer_orders|customer_wishlists]->(l IS orders|wishlists)-[IS list_items]->(p IS products) COLUMNS (c.name AS customer_name, p.name AS product_name, p.price AS price, x1.a AS a)) g+ ORDER BY g.customer_name, g.product_name; (1 row) diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index e761f09e057..80576b2f9f4 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -525,7 +525,8 @@ SELECT * FROM GRAPH_TABLE (g4 MATCH (s IS ptnv)-[e IS ptne]->(d IS ptnv) COLUMNS SELECT * FROM GRAPH_TABLE (g4 MATCH (s)-[e]-(d) WHERE s.id = 3 COLUMNS (s.val, e.val, d.val)) ORDER BY 1, 2, 3; SELECT * FROM GRAPH_TABLE (g4 MATCH (s WHERE s.id = 3)-[e]-(d) COLUMNS (s.val, e.val, d.val)) ORDER BY 1, 2, 3; --- ruleutils reverse parsing +-- GRAPH_TABLE in views + -- The query in the view definition is intentionally complex to test one view with many -- features like label disjunction, lateral references, WHERE clauses in graph -- patterns. @@ -534,8 +535,17 @@ SELECT g.* FROM x1, GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US' AND c.customer_id = x1.a) -[IS customer_orders | customer_wishlists ]-> (l IS orders | wishlists)-[ IS list_items]->(p IS products) - COLUMNS (c.name AS customer_name, p.name AS product_name, x1.a AS a)) g + COLUMNS (c.name AS customer_name, p.name AS product_name, p.price, x1.a AS a)) g ORDER BY customer_name, product_name; +-- Dropping properties or labels used by a view is not allowed +-- If these DDLs succeed, the pg_get_viewdef call below will throw cache lookup +-- error. +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE orders DROP LABEL orders; -- error +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers + ALTER LABEL customers DROP PROPERTIES (address); -- error +ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products + ALTER LABEL products DROP PROPERTIES (price); -- error +-- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); -- test view/graph nesting -- 2.34.1
