Re: Property graph: fix error handling when dropping non-existent label property
| От | Chao Li |
|---|---|
| Тема | Re: Property graph: fix error handling when dropping non-existent label property |
| Дата | |
| Msg-id | E3D6552E-DB26-47BC-9C7C-0F9CB936BF5B@gmail.com обсуждение |
| Ответ на | Re: Property graph: fix error handling when dropping non-existent label property (Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>) |
| Ответы |
Re: Property graph: fix error handling when dropping non-existent label property
|
| Список | pgsql-hackers |
> On Apr 24, 2026, at 20:38, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote: > > On Fri, Apr 24, 2026 at 1:03 PM 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=# 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 propertyexists. 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. 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. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Вложения
В списке pgsql-hackers по дате отправления: