From 9758fdad9c71e36e26a10350ad01102f7288d42e Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Thu, 2 Jul 2026 15:27:15 +0530 Subject: [PATCH v20260703 03/14] Report duplicate property and label names with a proper error Adding a property with the same name multiple times to a label on an element, either within a single PROPERTIES clause or across statements via ALTER PROPERTY GRAPH ... ADD PROPERTIES, previously failed with a unique-index violation on pg_propgraph_label_property. The same class of bug existed for labels: listing the same label multiple times on one element in CREATE PROPERTY GRAPH, or adding a label to an element that already has it via ALTER PROPERTY GRAPH ... ADD LABEL, failed with a unique-index violation on pg_propgraph_element_label. Detect the duplicates up front and raise a friendlier error in both cases. For properties, the cross-statement duplicate is caught by a syscache probe before insertion. The in-clause duplicate could have been caught by the same probe if we issued a CommandCounterIncrement() between property inserts, but that forces a catalog invalidation per property purely to detect a condition we can check for free on the in-memory target list. The list-based check is only needed when the properties are listed explicitly; when they are derived from the table's attributes, names are already unique. For labels, a single syscache probe on pg_propgraph_element_label suffices for both the in-clause and cross-statement cases because insert_element_record() already issues CommandCounterIncrement() between successive label inserts. Add regression tests exercising both bugs, and extend the existing alter-propgraph-concurrently isolation spec with permutations that add the same property, and the same label, from two concurrent sessions; the second session now sees the friendly duplicate-object error rather than a unique-index violation. Author: Ashutosh Bapat Reported by: Noah Misch Discussion: https://www.postgresql.org/message-id/20260630173053.51.noahmisch@microsoft.com --- src/backend/commands/propgraphcmds.c | 37 ++++++++++- .../expected/alter-propgraph-concurrently.out | 66 +++++++++++++++++++ .../specs/alter-propgraph-concurrently.spec | 14 ++++ .../expected/create_property_graph.out | 14 ++++ .../regress/sql/create_property_graph.sql | 12 ++++ 5 files changed, 140 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/propgraphcmds.c b/src/backend/commands/propgraphcmds.c index 62f91a08410..4c0fe763b5c 100644 --- a/src/backend/commands/propgraphcmds.c +++ b/src/backend/commands/propgraphcmds.c @@ -784,6 +784,13 @@ insert_label_record(Oid graphid, Oid peoid, const char *label) /* * Insert into pg_propgraph_element_label */ + if (SearchSysCacheExists2(PROPGRAPHELEMENTLABELELEMENTLABEL, + ObjectIdGetDatum(peoid), + ObjectIdGetDatum(labeloid))) + ereport(ERROR, + errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("label \"%s\" already exists", label)); + else { Relation rel; Datum values[Natts_pg_propgraph_element_label] = {0}; @@ -899,12 +906,30 @@ insert_property_records(Oid graphid, Oid ellabeloid, Oid pgerelid, const PropGra tp = transformTargetList(pstate, proplist, EXPR_KIND_PROPGRAPH_PROPERTY); assign_expr_collations(pstate, (Node *) tp); - foreach(lc, tp) + /* + * When properties are derived from the table's attributes, names are + * already unique. Reject duplicate property names within an explicit + * PROPERTIES clause. Do this after transformTargetList() so that any + * names derived by transformTargetList() are considered. + */ + if (!properties->all) { - TargetEntry *te = lfirst_node(TargetEntry, lc); + List *seen = NIL; - insert_property_record(graphid, ellabeloid, pgerelid, te->resname, te->expr); + foreach_node(TargetEntry, te, tp) + { + String *name = makeString(te->resname); + + if (list_member(seen, name)) + ereport(ERROR, + errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("property \"%s\" specified more than once", te->resname)); + seen = lappend(seen, name); + } } + + foreach_node(TargetEntry, te, tp) + insert_property_record(graphid, ellabeloid, pgerelid, te->resname, te->expr); } /* @@ -1001,6 +1026,12 @@ insert_property_record(Oid graphid, Oid ellabeloid, Oid pgerelid, const char *pr /* * Insert into pg_propgraph_label_property */ + if (SearchSysCacheExists2(PROPGRAPHLABELPROP, ObjectIdGetDatum(ellabeloid), + ObjectIdGetDatum(propoid))) + ereport(ERROR, + errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("property \"%s\" already exists", propname)); + else { Relation rel; Datum values[Natts_pg_propgraph_label_property] = {0}; diff --git a/src/test/isolation/expected/alter-propgraph-concurrently.out b/src/test/isolation/expected/alter-propgraph-concurrently.out index a75e75b3ca3..5bc8c58e298 100644 --- a/src/test/isolation/expected/alter-propgraph-concurrently.out +++ b/src/test/isolation/expected/alter-propgraph-concurrently.out @@ -125,3 +125,69 @@ CREATE PROPERTY GRAPH public.pgg1 ) (1 row) + +starting permutation: s1b s1addp s2addp s1c s2pgdef +step s1b: BEGIN; +step s1addp: ALTER PROPERTY GRAPH pgg1 ALTER VERTEX TABLE pgt1 ALTER LABEL pgl1 ADD PROPERTIES (a + b AS c); +step s2addp: ALTER PROPERTY GRAPH pgg1 ALTER VERTEX TABLE pgt1 ALTER LABEL pgl1 ADD PROPERTIES (a + b AS c); +step s1c: COMMIT; +step s2addp: <... completed> +ERROR: property "c" already exists +step s2pgdef: SELECT pg_get_propgraphdef('pgg1'::regclass); +pg_get_propgraphdef +----------------------------------------------------------------------------------------------------------------------------------------------------- +CREATE PROPERTY GRAPH public.pgg1 + VERTEX TABLES ( + pgt1 KEY (a) LABEL pgl1 PROPERTIES (b, (a + b) AS c) LABEL pgl2 PROPERTIES (a, b) + ) +(1 row) + + +starting permutation: s1b s1addp s2addp s1r s2pgdef +step s1b: BEGIN; +step s1addp: ALTER PROPERTY GRAPH pgg1 ALTER VERTEX TABLE pgt1 ALTER LABEL pgl1 ADD PROPERTIES (a + b AS c); +step s2addp: ALTER PROPERTY GRAPH pgg1 ALTER VERTEX TABLE pgt1 ALTER LABEL pgl1 ADD PROPERTIES (a + b AS c); +step s1r: ROLLBACK; +step s2addp: <... completed> +step s2pgdef: SELECT pg_get_propgraphdef('pgg1'::regclass); +pg_get_propgraphdef +----------------------------------------------------------------------------------------------------------------------------------------------------- +CREATE PROPERTY GRAPH public.pgg1 + VERTEX TABLES ( + pgt1 KEY (a) LABEL pgl1 PROPERTIES (b, (a + b) AS c) LABEL pgl2 PROPERTIES (a, b) + ) +(1 row) + + +starting permutation: s1b s1alabel s2alabel s1c s2pgdef +step s1b: BEGIN; +step s1alabel: ALTER PROPERTY GRAPH pgg1 ALTER VERTEX TABLE pgt1 ADD LABEL pgl3 PROPERTIES ALL COLUMNS; +step s2alabel: ALTER PROPERTY GRAPH pgg1 ALTER VERTEX TABLE pgt1 ADD LABEL pgl3 PROPERTIES ALL COLUMNS; +step s1c: COMMIT; +step s2alabel: <... completed> +ERROR: label "pgl3" already exists +step s2pgdef: SELECT pg_get_propgraphdef('pgg1'::regclass); +pg_get_propgraphdef +-------------------------------------------------------------------------------------------------------------------------------------------------------------------- +CREATE PROPERTY GRAPH public.pgg1 + VERTEX TABLES ( + pgt1 KEY (a) LABEL pgl1 PROPERTIES (b) LABEL pgl2 PROPERTIES (a, b) LABEL pgl3 PROPERTIES (a, b) + ) +(1 row) + + +starting permutation: s1b s1alabel s2alabel s1r s2pgdef +step s1b: BEGIN; +step s1alabel: ALTER PROPERTY GRAPH pgg1 ALTER VERTEX TABLE pgt1 ADD LABEL pgl3 PROPERTIES ALL COLUMNS; +step s2alabel: ALTER PROPERTY GRAPH pgg1 ALTER VERTEX TABLE pgt1 ADD LABEL pgl3 PROPERTIES ALL COLUMNS; +step s1r: ROLLBACK; +step s2alabel: <... completed> +step s2pgdef: SELECT pg_get_propgraphdef('pgg1'::regclass); +pg_get_propgraphdef +-------------------------------------------------------------------------------------------------------------------------------------------------------------------- +CREATE PROPERTY GRAPH public.pgg1 + VERTEX TABLES ( + pgt1 KEY (a) LABEL pgl1 PROPERTIES (b) LABEL pgl2 PROPERTIES (a, b) LABEL pgl3 PROPERTIES (a, b) + ) +(1 row) + diff --git a/src/test/isolation/specs/alter-propgraph-concurrently.spec b/src/test/isolation/specs/alter-propgraph-concurrently.spec index 50b7e0efe05..61f7de92fed 100644 --- a/src/test/isolation/specs/alter-propgraph-concurrently.spec +++ b/src/test/isolation/specs/alter-propgraph-concurrently.spec @@ -24,6 +24,8 @@ session s1 step s1b { BEGIN; } step s1dlabel { ALTER PROPERTY GRAPH pgg1 ALTER VERTEX TABLE pgt1 DROP LABEL pgl1; } step s1delem { ALTER PROPERTY GRAPH pgg1 DROP VERTEX TABLES (pgt1); } +step s1addp { ALTER PROPERTY GRAPH pgg1 ALTER VERTEX TABLE pgt1 ALTER LABEL pgl1 ADD PROPERTIES (a + b AS c); } +step s1alabel { ALTER PROPERTY GRAPH pgg1 ALTER VERTEX TABLE pgt1 ADD LABEL pgl3 PROPERTIES ALL COLUMNS; } step s1c { COMMIT; } step s1r { ROLLBACK; } @@ -56,3 +58,15 @@ permutation s1b s1delem s2addp s1c s2pgdef # s2addp succeeds since rollback leaves pgt1 in the graph permutation s1b s1delem s2addp s1r s2pgdef + +# s2addp fails since by then s1 has added the same property +permutation s1b s1addp s2addp s1c s2pgdef + +# s2addp succeeds since rollback leaves pgl1 without the added property +permutation s1b s1addp s2addp s1r s2pgdef + +# s2alabel fails since by then s1 has added the same label +permutation s1b s1alabel s2alabel s1c s2pgdef + +# s2alabel succeeds since rollback leaves pgt1 without the added label +permutation s1b s1alabel s2alabel s1r s2pgdef diff --git a/src/test/regress/expected/create_property_graph.out b/src/test/regress/expected/create_property_graph.out index f9960e5e0ae..5930d844319 100644 --- a/src/test/regress/expected/create_property_graph.out +++ b/src/test/regress/expected/create_property_graph.out @@ -200,6 +200,20 @@ CREATE PROPERTY GRAPH gx ); DROP PROPERTY GRAPH gx; DROP TABLE t1x, t2x; +CREATE PROPERTY GRAPH gx + VERTEX TABLES ( + t1 KEY (a) LABEL l1 PROPERTIES (a AS p, a AS p) -- duplicate property on label + ); +ERROR: property "p" specified more than once +ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t2 ALTER LABEL t2 ADD PROPERTIES (k * 2 AS i_j); -- duplicate property on label +ERROR: property "i_j" already exists +CREATE PROPERTY GRAPH gx + VERTEX TABLES ( + t1 KEY (a) LABEL l1 LABEL l1 -- duplicate label on element + ); +ERROR: label "l1" already exists +ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t3 ADD LABEL t3l1 NO PROPERTIES; -- duplicate label on element +ERROR: label "t3l1" already exists CREATE PROPERTY GRAPH gx VERTEX TABLES ( t1 KEY (a) LABEL l1 PROPERTIES (a, a AS aa), diff --git a/src/test/regress/sql/create_property_graph.sql b/src/test/regress/sql/create_property_graph.sql index b10d7191506..a4285ef8846 100644 --- a/src/test/regress/sql/create_property_graph.sql +++ b/src/test/regress/sql/create_property_graph.sql @@ -155,6 +155,18 @@ CREATE PROPERTY GRAPH gx DROP PROPERTY GRAPH gx; DROP TABLE t1x, t2x; +CREATE PROPERTY GRAPH gx + VERTEX TABLES ( + t1 KEY (a) LABEL l1 PROPERTIES (a AS p, a AS p) -- duplicate property on label + ); +ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t2 ALTER LABEL t2 ADD PROPERTIES (k * 2 AS i_j); -- duplicate property on label + +CREATE PROPERTY GRAPH gx + VERTEX TABLES ( + t1 KEY (a) LABEL l1 LABEL l1 -- duplicate label on element + ); +ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t3 ADD LABEL t3l1 NO PROPERTIES; -- duplicate label on element + CREATE PROPERTY GRAPH gx VERTEX TABLES ( t1 KEY (a) LABEL l1 PROPERTIES (a, a AS aa), -- 2.34.1