MIME-Version: 1.0
References: <tencent_43D9888041FA4FDE498C7BF1@qq.com>
 <CAExHW5v+0v8ZwDVdJF7p95cxXddLEjnQkT0RAVTPtW1NuaMO7A@mail.gmail.com>
 <tencent_5A70518C7FF4E4EE46C464D3@qq.com>
 <CAJTYsWV9uroc-1Gurh+UTbXQ2h3h-QRKYC+kHpAr9Y2GMyROBQ@mail.gmail.com>
 <CAJTYsWXgD9SpR2XCkot4N5UpLCwf+_EvoRTJTEwhEm=RFEF2WQ@mail.gmail.com>
 <CAEG8a3L9RTHSZ6uhyyRTB9T-hH4XZaRPCG_3zC3u-hBN0pfefQ@mail.gmail.com>
 <CAJTYsWVfexEKBDfmEGZ9YCs8+Eca_zk1E7nnpjmYQvnny2WViA@mail.gmail.com>
 <CAExHW5vyLHDw47+fs6Pt+CeFDN1Dhne+tHPjrJYOkkBpJ3Fhvw@mail.gmail.com>
 <CAEG8a3+LEi8dN_8=K7kKo-X85SvKcy6VSPW7mXQboNwx4G+Kqw@mail.gmail.com>
 <CAExHW5tyEXwxrb8-AZKx_G6fposscZRjEWs_31A06CYCC_hSLQ@mail.gmail.com>
In-Reply-To: 
 <CAExHW5tyEXwxrb8-AZKx_G6fposscZRjEWs_31A06CYCC_hSLQ@mail.gmail.com>
From: Junwang Zhao <zhjwpku@gmail.com>
Date: Mon, 18 May 2026 10:07:02 +0800
Message-ID: 
 <CAEG8a3KkZJaLNZcuEzUjJBropCpbRkYXyeoj+svaKvJVxQ9BcA@mail.gmail.com>
Subject: Re: (SQL/PGQ) cache lookup failed for label
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Cc: Ayush Tiwari <ayushtiwari.slg01@gmail.com>,
 zengman <zengman@halodbtech.com>,
	pgsql-hackers <pgsql-hackers@lists.postgresql.org>,
	Peter Eisentraut <peter@eisentraut.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAEG8a3KkZJaLNZcuEzUjJBropCpbRkYXyeoj%2BsvaKvJVxQ9BcA%40mail.gmail.com>
Precedence: bulk

On Mon, May 18, 2026 at 8:22=E2=80=AFAM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> On Sun, May 17, 2026 at 5:12=E2=80=AFPM 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=E2=80=AFPM 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=3D# 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=3D# 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 gra=
ph 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 becau=
se the
> >> > graph-level property still exists via products.name, and then verifi=
es that
> >> > dropping products.name is rejected with the dependency error from
> >> > customers_us.  That should cover GraphPropertyRef nodes reached thro=
ugh the
> >> > GRAPH_TABLE COLUMNS list, in addition to the existing label and grap=
h-pattern
> >> > property cases.
> >> >
> >> > I re-added customers.name afterward so the existing myshop graph rem=
ains
> >> > 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 =3D (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 =3D (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 l=
ist_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 mysho=
p.

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


--=20
Regards
Junwang Zhao