MIME-Version: 1.0
References: <1DA5D52A-4AFA-426E-83F7-42ED974D682B@gmail.com>
 <CAExHW5s1q7eyp4qJLPzCgnsE9tQ8uKdLyOwKt3cijdMR_0E+Cg@mail.gmail.com>
 <E3D6552E-DB26-47BC-9C7C-0F9CB936BF5B@gmail.com>
In-Reply-To: <E3D6552E-DB26-47BC-9C7C-0F9CB936BF5B@gmail.com>
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: Tue, 28 Apr 2026 20:31:56 +0530
Message-ID: 
 <CAExHW5s-4MHjo0wn=bNGQQd=Q+3SW_HGxJCFnoBF+hiy5ni8wA@mail.gmail.com>
Subject: Re: Property graph: fix error handling when dropping non-existent
 label property
To: Chao Li <li.evan.chao@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAExHW5s-4MHjo0wn%3DbNGQQd%3DQ%2B3SW_HGxJCFnoBF%2Bhiy5ni8wA%40mail.gmail.com>
Precedence: bulk

On Sun, Apr 26, 2026 at 12:51=E2=80=AFPM Chao Li <li.evan.chao@gmail.com> w=
rote:
>
>
>
> > On Apr 24, 2026, at 20:38, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com=
> wrote:
> >
> > On Fri, Apr 24, 2026 at 1:03=E2=80=AFPM Chao Li <li.evan.chao@gmail.com=
> wrote:
> >>
> >> Hi,
> >>
> >> I am testing graph tables today and noticed an improper error message =
when dropping a property.
> >>
> >> Here is a simple repro:
> >> ```
> >> evantest=3D# CREATE TABLE t1 (a int PRIMARY KEY, b int);
> >> CREATE TABLE
> >> evantest=3D#
> >> evantest=3D# CREATE PROPERTY GRAPH g
> >> evantest-#     VERTEX TABLES (
> >> evantest(#         t1
> >> evantest(#             LABEL l1 PROPERTIES (a AS p1)
> >> evantest(#             LABEL l2 PROPERTIES (b AS p2)
> >> evantest(#     );
> >> CREATE PROPERTY GRAPH
> >> evantest=3D#
> >> evantest=3D# ALTER PROPERTY GRAPH g
> >> evantest-#     ALTER VERTEX TABLE t1
> >> evantest-#     ALTER LABEL l1 DROP PROPERTIES (p2);
> >> ERROR:  could not find tuple for label property 0
> >> ```
> >>
> >> This does not look like a normal user-facing SQL error message.
> >>
> >> Looking into the code, AlterPropGraph() checks whether the property ex=
ists in the graph, but does not check if label property exists. As a result=
, InvalidOid can be passed to ObjectAddressSet() and then to performDeletio=
n().
> >>
> >> The fix is simple, just check the label-property OID before trying to =
delete it. I also added a regress test.
> >>
> >> With the patch, the error becomes:
> >> ```
> >> evantest=3D# ALTER PROPERTY GRAPH g
> >> evantest-#     ALTER VERTEX TABLE t1
> >> evantest-#     ALTER LABEL l1 DROP PROPERTIES (p2);
> >> ERROR:  property graph "g" element "t1" label "l1" has no property "p2=
"
> >> ```
> >
> > Thanks for the report. Agree that we need to provide a correct error
> > message. The code changes look good to me.
>
> Cool! Thanks for reviewing and confirming.
>
> > However, the way you have
> > written this code is different from similar code earlier in the
> > function. Either your code should match that style or that code should
> > be changed to your style. I like your way - reduces code a bit and
> > does not repeat ereport. I also noticed that the code to fetch the
> > element label oid from element_alias and label name is repeated along
> > with the ereport in a few places. I think we could instead write a
> > function to do that and call that function in those places. When
> > writing the function, we could change that code to use your style.
>
> I updated the nearby code to align with my style. PFA v2.

Looks good to me. However, I did change OidIsValid() and !OidIsValid()
back to (oid) and (!oid) conditions to be consistent with the rest of
the code.

I tried to deduplicate the code which finds element labelid from given
element alias and label name. But there is one place where we also use
the label oid and or the element oid that the code extracts. So it
didn't end up being a lot of improvement.

Since this patch conflicts with the patch at [1], I am posting this
patch along with the patch there on that thread so that both can be
committed without causing a merge conflict. If necessary we can
continue discussion here.

[1] https://www.postgresql.org/message-id/CAExHW5uThebnwqyWHrsF2Z+no-fT8VrM=
0Yj+RU7YWrYzdavUZg@mail.gmail.com


--=20
Best Wishes,
Ashutosh Bapat