From 33a937ebc654a18472fe96a33310a945af3768bb Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Thu, 2 Jul 2026 15:27:15 +0530 Subject: [PATCH v20260716 1/4] 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. Author: Ashutosh Bapat Reported by: Noah Misch Discussion: https://www.postgresql.org/message-id/20260630173053.51.noahmisch@microsoft.com --- src/backend/commands/propgraphcmds.c | 151 +++++++++++++----- .../expected/create_property_graph.out | 14 ++ .../regress/sql/create_property_graph.sql | 12 ++ 3 files changed, 135 insertions(+), 42 deletions(-) diff --git a/src/backend/commands/propgraphcmds.c b/src/backend/commands/propgraphcmds.c index c87c519b032..fe76af6461a 100644 --- a/src/backend/commands/propgraphcmds.c +++ b/src/backend/commands/propgraphcmds.c @@ -82,7 +82,9 @@ static void propgraph_edge_get_ref_keys(ParseState *pstate, const List *keycols, static AttrNumber *array_from_column_list(ParseState *pstate, const List *colnames, int location, Relation element_rel); static ArrayType *array_from_attnums(int numattrs, const AttrNumber *attnums); static Oid insert_element_record(ObjectAddress pgaddress, struct element_info *einfo); +static void precheck_label_record(Oid graphid, Oid peoid, const char *label); static Oid insert_label_record(Oid graphid, Oid peoid, const char *label); +static void precheck_property_records(Oid graphid, Oid ellabeloid, const List *tp, bool has_explicit_list); static void insert_property_records(Oid graphid, Oid ellabeloid, Oid pgerelid, const PropGraphProperties *properties); static void insert_property_record(Oid graphid, Oid ellabeloid, Oid pgerelid, const char *propname, const Expr *expr); static void check_element_properties(Oid peoid); @@ -709,12 +711,11 @@ insert_element_record(ObjectAddress pgaddress, struct element_info *einfo) foreach(lc, einfo->labels) { PropGraphLabelAndProperties *lp = lfirst_node(PropGraphLabelAndProperties, lc); + const char *labelname = lp->label ? lp->label : einfo->aliasname; Oid ellabeloid; - if (lp->label) - ellabeloid = insert_label_record(graphid, peoid, lp->label); - else - ellabeloid = insert_label_record(graphid, peoid, einfo->aliasname); + precheck_label_record(graphid, peoid, labelname); + ellabeloid = insert_label_record(graphid, peoid, labelname); insert_property_records(graphid, ellabeloid, einfo->relid, lp->properties); CommandCounterIncrement(); @@ -728,6 +729,7 @@ insert_element_record(ObjectAddress pgaddress, struct element_info *einfo) pr->all = true; pr->location = -1; + precheck_label_record(graphid, peoid, einfo->aliasname); ellabeloid = insert_label_record(graphid, peoid, einfo->aliasname); insert_property_records(graphid, ellabeloid, einfo->relid, pr); } @@ -816,6 +818,27 @@ insert_label_record(Oid graphid, Oid peoid, const char *label) return ellabeloid; } +/* + * Throw an error if the label already exists for the given element. + */ +static void +precheck_label_record(Oid graphid, Oid peoid, const char *label) +{ + Oid labeloid; + + labeloid = GetSysCacheOid2(PROPGRAPHLABELNAME, + Anum_pg_propgraph_label_oid, + ObjectIdGetDatum(graphid), + CStringGetDatum(label)); + if (OidIsValid(labeloid) && + SearchSysCacheExists2(PROPGRAPHELEMENTLABELELEMENTLABEL, + ObjectIdGetDatum(peoid), + ObjectIdGetDatum(labeloid))) + ereport(ERROR, + errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("label \"%s\" already exists", label)); +} + /* * Insert records for properties into the pg_propgraph_property catalog. */ @@ -901,11 +924,85 @@ insert_property_records(Oid graphid, Oid ellabeloid, Oid pgerelid, const PropGra resolveTargetListUnknowns(pstate, tp); assign_expr_collations(pstate, (Node *) tp); - foreach(lc, tp) - { - TargetEntry *te = lfirst_node(TargetEntry, lc); + /* + * Instruct the precheck to check for duplicates in the transformed target + * list. + */ + precheck_property_records(graphid, ellabeloid, tp, !properties->all); + foreach_node(TargetEntry, te, tp) insert_property_record(graphid, ellabeloid, pgerelid, te->resname, te->expr); +} + +/* + * Reject duplicate property names for this element label, and reject + * property expressions whose type or collation does not match the existing + * entry in pg_propgraph_property. + * + * When has_explicit_list is true, catch duplicates within the transformed + * target list itself. + */ +static void +precheck_property_records(Oid graphid, Oid ellabeloid, const List *tp, bool has_explicit_list) +{ + List *seen = NIL; + + foreach_node(TargetEntry, te, tp) + { + HeapTuple pgptup; + + if (has_explicit_list) + { + 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); + } + + pgptup = SearchSysCache2(PROPGRAPHPROPNAME, + ObjectIdGetDatum(graphid), + CStringGetDatum(te->resname)); + if (HeapTupleIsValid(pgptup)) + { + Form_pg_propgraph_property pgpform = (Form_pg_propgraph_property) GETSTRUCT(pgptup); + Oid propoid = pgpform->oid; + Oid proptypid = pgpform->pgptypid; + int32 proptypmod = pgpform->pgptypmod; + Oid propcollation = pgpform->pgpcollation; + Oid exprtypid = exprType((const Node *) te->expr); + int32 exprtypmod = exprTypmod((const Node *) te->expr); + Oid exprcollation = exprCollation((const Node *) te->expr); + + ReleaseSysCache(pgptup); + + if (SearchSysCacheExists2(PROPGRAPHLABELPROP, + ObjectIdGetDatum(ellabeloid), + ObjectIdGetDatum(propoid))) + ereport(ERROR, + errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("property \"%s\" already exists", te->resname)); + + if (proptypid != exprtypid || proptypmod != exprtypmod) + ereport(ERROR, + errcode(ERRCODE_SYNTAX_ERROR), + errmsg("property \"%s\" data type mismatch: %s vs. %s", + te->resname, + format_type_with_typemod(proptypid, proptypmod), + format_type_with_typemod(exprtypid, exprtypmod)), + errdetail("In a property graph, a property of the same name has to have the same data type in each label.")); + + if (propcollation != exprcollation) + ereport(ERROR, + errcode(ERRCODE_SYNTAX_ERROR), + errmsg("property \"%s\" collation mismatch: %s vs. %s", + te->resname, + get_collation_name(propcollation), + get_collation_name(exprcollation)), + errdetail("In a property graph, a property of the same name has to have the same collation in each label.")); + } } } @@ -964,41 +1061,6 @@ insert_property_record(Oid graphid, Oid ellabeloid, Oid pgerelid, const char *pr table_close(rel, NoLock); } - else - { - HeapTuple pgptup = SearchSysCache1(PROPGRAPHPROPOID, ObjectIdGetDatum(propoid)); - Form_pg_propgraph_property pgpform = (Form_pg_propgraph_property) GETSTRUCT(pgptup); - Oid proptypid = pgpform->pgptypid; - int32 proptypmod = pgpform->pgptypmod; - Oid propcollation = pgpform->pgpcollation; - - ReleaseSysCache(pgptup); - - /* - * Check that in the graph, all properties with the same name have the - * same type (independent of which label they are on). (See SQL/PGQ - * subclause "Consistency check of a tabular property graph - * descriptor".) - */ - if (proptypid != exprtypid || proptypmod != exprtypmod) - { - ereport(ERROR, - errcode(ERRCODE_SYNTAX_ERROR), - errmsg("property \"%s\" data type mismatch: %s vs. %s", - propname, format_type_with_typemod(proptypid, proptypmod), format_type_with_typemod(exprtypid, exprtypmod)), - errdetail("In a property graph, a property of the same name has to have the same data type in each label.")); - } - - /* Similarly for collation */ - if (propcollation != exprcollation) - { - ereport(ERROR, - errcode(ERRCODE_SYNTAX_ERROR), - errmsg("property \"%s\" collation mismatch: %s vs. %s", - propname, get_collation_name(propcollation), get_collation_name(exprcollation)), - errdetail("In a property graph, a property of the same name has to have the same collation in each label.")); - } - } /* * Insert into pg_propgraph_label_property @@ -1486,6 +1548,11 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt) pgerelid = get_element_relid(peoid); + /* + * Since we increment command counter after inserting every label, + * duplicate label names in the command are caught by label precheck. + */ + precheck_label_record(pgrelid, peoid, lp->label); ellabeloid = insert_label_record(pgrelid, peoid, lp->label); insert_property_records(pgrelid, ellabeloid, pgerelid, lp->properties); diff --git a/src/test/regress/expected/create_property_graph.out b/src/test/regress/expected/create_property_graph.out index 7387751eac2..4a8b3d91219 100644 --- a/src/test/regress/expected/create_property_graph.out +++ b/src/test/regress/expected/create_property_graph.out @@ -207,6 +207,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 3494390b923..1667896f558 100644 --- a/src/test/regress/sql/create_property_graph.sql +++ b/src/test/regress/sql/create_property_graph.sql @@ -161,6 +161,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), base-commit: f4f77f557bb1a173f82be42770a28758732f4cc0 -- 2.34.1