Re: Property graph: fix error handling when dropping non-existent label property

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Property graph: fix error handling when dropping non-existent label property
Date: 2026-04-24 12:38:07
Message-ID: CAExHW5s1q7eyp4qJLPzCgnsE9tQ8uKdLyOwKt3cijdMR_0E+Cg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Apr 24, 2026 at 1:03 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)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=# CREATE TABLE t1 (a int PRIMARY KEY, b int);
> CREATE TABLE
> evantest=#
> evantest=# 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=#
> evantest=# 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 exists in the graph, but does not check if label property exists. As a result, InvalidOid can be passed to ObjectAddressSet() and then to performDeletion().
>
> 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=# 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. 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.

--
Best Wishes,
Ashutosh Bapat

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2026-04-24 12:43:43 Re: Redundant/mis-use of _(x) gettext macro?
Previous Message Álvaro Herrera 2026-04-24 12:14:30 Re: Redundant/mis-use of _(x) gettext macro?