From 048e06179cec4b2fefed653a49bd3fc9b106083b Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Thu, 16 Jul 2026 15:30:19 +0530 Subject: [PATCH v20260716] 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 diff --git a/src/backend/commands/propgraphcmds.c b/src/backend/commands/propgraphcmds.c index c87c519b032..9c9f4f2b299 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}; @@ -901,12 +908,30 @@ insert_property_records(Oid graphid, Oid ellabeloid, Oid pgerelid, const PropGra resolveTargetListUnknowns(pstate, tp); 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); } /* @@ -1003,6 +1028,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/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), --- src/backend/commands/propgraphcmds.c | 487 +++++++++++------- .../expected/create_property_graph.out | 14 + .../regress/sql/create_property_graph.sql | 12 + 3 files changed, 322 insertions(+), 191 deletions(-) diff --git a/src/backend/commands/propgraphcmds.c b/src/backend/commands/propgraphcmds.c index c87c519b032..2938e8bd9ca 100644 --- a/src/backend/commands/propgraphcmds.c +++ b/src/backend/commands/propgraphcmds.c @@ -82,9 +82,20 @@ 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 Oid insert_label_record(Oid graphid, Oid peoid, const char *label); -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 Oid insert_label_record(Oid graphid, const char *label); +static Oid insert_property_record(Oid graphid, const char *propname, + Oid typid, int32 typmod, Oid collation); +static Oid insert_element_label_record(Oid peoid, Oid labeloid); +static void insert_label_property_record(Oid ellabeloid, Oid propoid, Oid pgerelid, + const Expr *expr); +static Oid add_element(ObjectAddress pgaddress, struct element_info *einfo); +static Oid add_element_label(Oid graphid, Oid peoid, const char *label); +static Oid add_element_label_properties(Oid graphid, Oid peoid, Oid pgerelid, + const char *label, + const PropGraphProperties *properties); +static void add_label_properties(Oid graphid, Oid ellabeloid, Oid pgerelid, const PropGraphProperties *properties); +static void add_label_property(Oid graphid, Oid ellabeloid, Oid pgerelid, + const char *propname, const Expr *expr); static void check_element_properties(Oid peoid); static void check_element_label_properties(Oid ellabeloid); static void check_all_labels_properties(Oid pgrelid); @@ -268,7 +279,7 @@ CreatePropGraph(ParseState *pstate, const CreatePropGraphStmt *stmt) struct element_info *vinfo = lfirst(lc); Oid peoid; - peoid = insert_element_record(pgaddress, vinfo); + peoid = add_element(pgaddress, vinfo); element_oids = lappend_oid(element_oids, peoid); } @@ -303,7 +314,7 @@ CreatePropGraph(ParseState *pstate, const CreatePropGraphStmt *stmt) Assert(einfo->destvertexid); Assert(einfo->srcrelid); Assert(einfo->destrelid); - peoid = insert_element_record(pgaddress, einfo); + peoid = add_element(pgaddress, einfo); element_oids = lappend_oid(element_oids, peoid); } @@ -605,8 +616,8 @@ array_of_opers_to_objectaddrs(ArrayType *arr, ObjectAddresses *addrs) } /* - * Insert a record for an element into the pg_propgraph_element catalog. Also - * inserts labels and properties into their respective catalogs. + * Insert a record into pg_propgraph_element catalog with the required + * dependencies. */ static Oid insert_element_record(ObjectAddress pgaddress, struct element_info *einfo) @@ -702,6 +713,182 @@ insert_element_record(ObjectAddress pgaddress, struct element_info *einfo) table_close(rel, NoLock); + return peoid; +} + +/* + * Insert a record into pg_propgraph_label catalog for given label with required + * dependencies. + */ +static Oid +insert_label_record(Oid graphid, const char *label) +{ + Relation rel; + Datum values[Natts_pg_propgraph_label] = {0}; + bool nulls[Natts_pg_propgraph_label] = {0}; + NameData labelname; + Oid labeloid; + HeapTuple tup; + ObjectAddress myself; + ObjectAddress referenced; + + rel = table_open(PropgraphLabelRelationId, RowExclusiveLock); + + labeloid = GetNewOidWithIndex(rel, PropgraphLabelObjectIndexId, Anum_pg_propgraph_label_oid); + values[Anum_pg_propgraph_label_oid - 1] = ObjectIdGetDatum(labeloid); + values[Anum_pg_propgraph_label_pglpgid - 1] = ObjectIdGetDatum(graphid); + namestrcpy(&labelname, label); + values[Anum_pg_propgraph_label_pgllabel - 1] = NameGetDatum(&labelname); + + tup = heap_form_tuple(RelationGetDescr(rel), values, nulls); + CatalogTupleInsert(rel, tup); + heap_freetuple(tup); + + ObjectAddressSet(myself, PropgraphLabelRelationId, labeloid); + ObjectAddressSet(referenced, RelationRelationId, graphid); + recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); + + table_close(rel, NoLock); + + return labeloid; +} + +/* + * Insert a record into pg_propgraph_property catalog for the given property + * with required dependencies. + */ +static Oid +insert_property_record(Oid graphid, const char *propname, + Oid typid, int32 typmod, Oid collation) +{ + Relation rel; + NameData propnamedata; + Datum values[Natts_pg_propgraph_property] = {0}; + bool nulls[Natts_pg_propgraph_property] = {0}; + Oid propoid; + HeapTuple tup; + ObjectAddress myself; + ObjectAddress referenced; + + rel = table_open(PropgraphPropertyRelationId, RowExclusiveLock); + + propoid = GetNewOidWithIndex(rel, PropgraphPropertyObjectIndexId, Anum_pg_propgraph_property_oid); + values[Anum_pg_propgraph_property_oid - 1] = ObjectIdGetDatum(propoid); + values[Anum_pg_propgraph_property_pgppgid - 1] = ObjectIdGetDatum(graphid); + namestrcpy(&propnamedata, propname); + values[Anum_pg_propgraph_property_pgpname - 1] = NameGetDatum(&propnamedata); + values[Anum_pg_propgraph_property_pgptypid - 1] = ObjectIdGetDatum(typid); + values[Anum_pg_propgraph_property_pgptypmod - 1] = Int32GetDatum(typmod); + values[Anum_pg_propgraph_property_pgpcollation - 1] = ObjectIdGetDatum(collation); + + tup = heap_form_tuple(RelationGetDescr(rel), values, nulls); + CatalogTupleInsert(rel, tup); + heap_freetuple(tup); + + ObjectAddressSet(myself, PropgraphPropertyRelationId, propoid); + ObjectAddressSet(referenced, RelationRelationId, graphid); + recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); + ObjectAddressSet(referenced, TypeRelationId, typid); + recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); + if (OidIsValid(collation) && collation != DEFAULT_COLLATION_OID) + { + ObjectAddressSet(referenced, CollationRelationId, collation); + recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); + } + + table_close(rel, NoLock); + + return propoid; +} + +/* + * Insert a record into pg_propgraph_element_label catalog associating the given label + * with the given element with required dependencies. + */ +static Oid +insert_element_label_record(Oid peoid, Oid labeloid) +{ + Relation rel; + Datum values[Natts_pg_propgraph_element_label] = {0}; + bool nulls[Natts_pg_propgraph_element_label] = {0}; + Oid ellabeloid; + HeapTuple tup; + ObjectAddress myself; + ObjectAddress referenced; + + rel = table_open(PropgraphElementLabelRelationId, RowExclusiveLock); + + ellabeloid = GetNewOidWithIndex(rel, PropgraphElementLabelObjectIndexId, Anum_pg_propgraph_element_label_oid); + values[Anum_pg_propgraph_element_label_oid - 1] = ObjectIdGetDatum(ellabeloid); + values[Anum_pg_propgraph_element_label_pgellabelid - 1] = ObjectIdGetDatum(labeloid); + values[Anum_pg_propgraph_element_label_pgelelid - 1] = ObjectIdGetDatum(peoid); + + tup = heap_form_tuple(RelationGetDescr(rel), values, nulls); + CatalogTupleInsert(rel, tup); + heap_freetuple(tup); + + ObjectAddressSet(myself, PropgraphElementLabelRelationId, ellabeloid); + ObjectAddressSet(referenced, PropgraphLabelRelationId, labeloid); + recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); + ObjectAddressSet(referenced, PropgraphElementRelationId, peoid); + recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); + + table_close(rel, NoLock); + + return ellabeloid; +} + +/* + * Insert a record into pg_propgraph_label_property catalog associating the given + * property with the given element label with required dependencies. + */ +static void +insert_label_property_record(Oid ellabeloid, Oid propoid, Oid pgerelid, + const Expr *expr) +{ + Relation rel; + Datum values[Natts_pg_propgraph_label_property] = {0}; + bool nulls[Natts_pg_propgraph_label_property] = {0}; + Oid plpoid; + HeapTuple tup; + ObjectAddress myself; + ObjectAddress referenced; + + rel = table_open(PropgraphLabelPropertyRelationId, RowExclusiveLock); + + plpoid = GetNewOidWithIndex(rel, PropgraphLabelPropertyObjectIndexId, Anum_pg_propgraph_label_property_oid); + values[Anum_pg_propgraph_label_property_oid - 1] = ObjectIdGetDatum(plpoid); + values[Anum_pg_propgraph_label_property_plppropid - 1] = ObjectIdGetDatum(propoid); + values[Anum_pg_propgraph_label_property_plpellabelid - 1] = ObjectIdGetDatum(ellabeloid); + values[Anum_pg_propgraph_label_property_plpexpr - 1] = CStringGetTextDatum(nodeToString(expr)); + + tup = heap_form_tuple(RelationGetDescr(rel), values, nulls); + CatalogTupleInsert(rel, tup); + heap_freetuple(tup); + + ObjectAddressSet(myself, PropgraphLabelPropertyRelationId, plpoid); + ObjectAddressSet(referenced, PropgraphPropertyRelationId, propoid); + recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); + ObjectAddressSet(referenced, PropgraphElementLabelRelationId, ellabeloid); + recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); + + recordDependencyOnSingleRelExpr(&myself, (Node *) copyObject(expr), pgerelid, DEPENDENCY_NORMAL, DEPENDENCY_NORMAL, false); + + table_close(rel, NoLock); +} + +/* + * Add a property graph element associating it with the given labels and + * properties. + */ +static Oid +add_element(ObjectAddress pgaddress, struct element_info *einfo) +{ + Oid graphid = pgaddress.objectId; + Oid peoid; + + peoid = insert_element_record(pgaddress, einfo); + if (einfo->labels) { ListCell *lc; @@ -709,109 +896,76 @@ insert_element_record(ObjectAddress pgaddress, struct element_info *einfo) foreach(lc, einfo->labels) { PropGraphLabelAndProperties *lp = lfirst_node(PropGraphLabelAndProperties, lc); - Oid ellabeloid; - if (lp->label) - ellabeloid = insert_label_record(graphid, peoid, lp->label); - else - ellabeloid = insert_label_record(graphid, peoid, einfo->aliasname); - insert_property_records(graphid, ellabeloid, einfo->relid, lp->properties); + add_element_label_properties(graphid, peoid, einfo->relid, + lp->label ? lp->label : einfo->aliasname, + lp->properties); CommandCounterIncrement(); } } else { - Oid ellabeloid; PropGraphProperties *pr = makeNode(PropGraphProperties); pr->all = true; pr->location = -1; - ellabeloid = insert_label_record(graphid, peoid, einfo->aliasname); - insert_property_records(graphid, ellabeloid, einfo->relid, pr); + add_element_label_properties(graphid, peoid, einfo->relid, einfo->aliasname, pr); } return peoid; } /* - * Insert records for a label into the pg_propgraph_label and - * pg_propgraph_element_label catalogs, and register dependencies. + * Add the given label to the given property graph element. + * + * ... creating a new pg_propgraph_label record if necessary. Throw an error if + * the label already exists for the element. * - * Returns the OID of the new pg_propgraph_element_label record. + * Returns the OID of the new pg_propgraph_element_label row. */ static Oid -insert_label_record(Oid graphid, Oid peoid, const char *label) +add_element_label(Oid graphid, Oid peoid, const char *label) { Oid labeloid; - Oid ellabeloid; - /* - * Insert into pg_propgraph_label if not already existing. - */ - labeloid = GetSysCacheOid2(PROPGRAPHLABELNAME, Anum_pg_propgraph_label_oid, ObjectIdGetDatum(graphid), CStringGetDatum(label)); - if (!labeloid) + labeloid = GetSysCacheOid2(PROPGRAPHLABELNAME, + Anum_pg_propgraph_label_oid, + ObjectIdGetDatum(graphid), + CStringGetDatum(label)); + if (OidIsValid(labeloid)) { - Relation rel; - Datum values[Natts_pg_propgraph_label] = {0}; - bool nulls[Natts_pg_propgraph_label] = {0}; - NameData labelname; - HeapTuple tup; - ObjectAddress myself; - ObjectAddress referenced; - - rel = table_open(PropgraphLabelRelationId, RowExclusiveLock); - - labeloid = GetNewOidWithIndex(rel, PropgraphLabelObjectIndexId, Anum_pg_propgraph_label_oid); - values[Anum_pg_propgraph_label_oid - 1] = ObjectIdGetDatum(labeloid); - values[Anum_pg_propgraph_label_pglpgid - 1] = ObjectIdGetDatum(graphid); - namestrcpy(&labelname, label); - values[Anum_pg_propgraph_label_pgllabel - 1] = NameGetDatum(&labelname); - - tup = heap_form_tuple(RelationGetDescr(rel), values, nulls); - CatalogTupleInsert(rel, tup); - heap_freetuple(tup); - - ObjectAddressSet(myself, PropgraphLabelRelationId, labeloid); - - ObjectAddressSet(referenced, RelationRelationId, graphid); - recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); - - table_close(rel, NoLock); + if (SearchSysCacheExists2(PROPGRAPHELEMENTLABELELEMENTLABEL, + ObjectIdGetDatum(peoid), + ObjectIdGetDatum(labeloid))) + ereport(ERROR, + errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("label \"%s\" already exists", label)); } + else + labeloid = insert_label_record(graphid, label); - /* - * Insert into pg_propgraph_element_label - */ - { - Relation rel; - Datum values[Natts_pg_propgraph_element_label] = {0}; - bool nulls[Natts_pg_propgraph_element_label] = {0}; - HeapTuple tup; - ObjectAddress myself; - ObjectAddress referenced; - - rel = table_open(PropgraphElementLabelRelationId, RowExclusiveLock); - - ellabeloid = GetNewOidWithIndex(rel, PropgraphElementLabelObjectIndexId, Anum_pg_propgraph_element_label_oid); - values[Anum_pg_propgraph_element_label_oid - 1] = ObjectIdGetDatum(ellabeloid); - values[Anum_pg_propgraph_element_label_pgellabelid - 1] = ObjectIdGetDatum(labeloid); - values[Anum_pg_propgraph_element_label_pgelelid - 1] = ObjectIdGetDatum(peoid); - - tup = heap_form_tuple(RelationGetDescr(rel), values, nulls); - CatalogTupleInsert(rel, tup); - heap_freetuple(tup); - - ObjectAddressSet(myself, PropgraphElementLabelRelationId, ellabeloid); + return insert_element_label_record(peoid, labeloid); +} - ObjectAddressSet(referenced, PropgraphLabelRelationId, labeloid); - recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); - ObjectAddressSet(referenced, PropgraphElementRelationId, peoid); - recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); +/* + * Add a label to a property graph element along with its properties. + * + * pgerelid is the OID of the element's underlying relation required to resolve + * column references in the property expressions. + * + * Returns the OID of the pg_propgraph_element_label. + */ +static Oid +add_element_label_properties(Oid graphid, Oid peoid, Oid pgerelid, + const char *label, + const PropGraphProperties *properties) +{ + Oid ellabeloid; - table_close(rel, NoLock); - } + ellabeloid = add_element_label(graphid, peoid, label); + add_label_properties(graphid, ellabeloid, pgerelid, properties); return ellabeloid; } @@ -820,7 +974,7 @@ insert_label_record(Oid graphid, Oid peoid, const char *label) * Insert records for properties into the pg_propgraph_property catalog. */ static void -insert_property_records(Oid graphid, Oid ellabeloid, Oid pgerelid, const PropGraphProperties *properties) +add_label_properties(Oid graphid, Oid ellabeloid, Oid pgerelid, const PropGraphProperties *properties) { List *proplist = NIL; ParseState *pstate; @@ -901,141 +1055,94 @@ insert_property_records(Oid graphid, Oid ellabeloid, Oid pgerelid, const PropGra resolveTargetListUnknowns(pstate, tp); assign_expr_collations(pstate, (Node *) tp); - foreach(lc, tp) + /* + * Reject duplicate property names within an explicit PROPERTIES clause. + * add_label_property()'s syscache probe cannot see property rows added + * earlier in this same call, since there is no CommandCounterIncrement() + * between them. Done after transformTargetList() so that names it derives + * are considered. When properties are derived from the table's + * attributes, names are already unique. + */ + 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) + add_label_property(graphid, ellabeloid, pgerelid, te->resname, te->expr); } /* - * Insert records for a property into the pg_propgraph_property and - * pg_propgraph_label_property catalogs, and register dependencies. + * Add a property to an element label + * + * ... creating a new pg_propgraph_property record if necessary. Throw an error + * if the property already exists for the element label, or if the property is + * defined already with a different type or collation. */ static void -insert_property_record(Oid graphid, Oid ellabeloid, Oid pgerelid, const char *propname, const Expr *expr) +add_label_property(Oid graphid, Oid ellabeloid, Oid pgerelid, + const char *propname, const Expr *expr) { - Oid propoid; Oid exprtypid = exprType((const Node *) expr); int32 exprtypmod = exprTypmod((const Node *) expr); Oid exprcollation = exprCollation((const Node *) expr); + HeapTuple pgptup; + Oid propoid; - /* - * Insert into pg_propgraph_property if not already existing. - */ - propoid = GetSysCacheOid2(PROPGRAPHPROPNAME, Anum_pg_propgraph_property_oid, ObjectIdGetDatum(graphid), CStringGetDatum(propname)); - if (!OidIsValid(propoid)) - { - Relation rel; - NameData propnamedata; - Datum values[Natts_pg_propgraph_property] = {0}; - bool nulls[Natts_pg_propgraph_property] = {0}; - HeapTuple tup; - ObjectAddress myself; - ObjectAddress referenced; - - rel = table_open(PropgraphPropertyRelationId, RowExclusiveLock); - - propoid = GetNewOidWithIndex(rel, PropgraphPropertyObjectIndexId, Anum_pg_propgraph_property_oid); - values[Anum_pg_propgraph_property_oid - 1] = ObjectIdGetDatum(propoid); - values[Anum_pg_propgraph_property_pgppgid - 1] = ObjectIdGetDatum(graphid); - namestrcpy(&propnamedata, propname); - values[Anum_pg_propgraph_property_pgpname - 1] = NameGetDatum(&propnamedata); - values[Anum_pg_propgraph_property_pgptypid - 1] = ObjectIdGetDatum(exprtypid); - values[Anum_pg_propgraph_property_pgptypmod - 1] = Int32GetDatum(exprtypmod); - values[Anum_pg_propgraph_property_pgpcollation - 1] = ObjectIdGetDatum(exprcollation); - - tup = heap_form_tuple(RelationGetDescr(rel), values, nulls); - CatalogTupleInsert(rel, tup); - heap_freetuple(tup); - - ObjectAddressSet(myself, PropgraphPropertyRelationId, propoid); - - ObjectAddressSet(referenced, RelationRelationId, graphid); - recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); - ObjectAddressSet(referenced, TypeRelationId, exprtypid); - recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); - if (OidIsValid(exprcollation) && exprcollation != DEFAULT_COLLATION_OID) - { - ObjectAddressSet(referenced, CollationRelationId, exprcollation); - recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); - } - - table_close(rel, NoLock); - } - else + pgptup = SearchSysCache2(PROPGRAPHPROPNAME, + ObjectIdGetDatum(graphid), + CStringGetDatum(propname)); + if (HeapTupleIsValid(pgptup)) { - 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; + propoid = pgpform->oid; 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 (SearchSysCacheExists2(PROPGRAPHLABELPROP, + ObjectIdGetDatum(ellabeloid), + ObjectIdGetDatum(propoid))) + ereport(ERROR, + errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("property \"%s\" already exists", propname)); + 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)), + 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)), + 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.")); - } } + else + propoid = insert_property_record(graphid, propname, + exprtypid, exprtypmod, exprcollation); - /* - * Insert into pg_propgraph_label_property - */ - { - Relation rel; - Datum values[Natts_pg_propgraph_label_property] = {0}; - bool nulls[Natts_pg_propgraph_label_property] = {0}; - Oid plpoid; - HeapTuple tup; - ObjectAddress myself; - ObjectAddress referenced; - - rel = table_open(PropgraphLabelPropertyRelationId, RowExclusiveLock); - - plpoid = GetNewOidWithIndex(rel, PropgraphLabelPropertyObjectIndexId, Anum_pg_propgraph_label_property_oid); - values[Anum_pg_propgraph_label_property_oid - 1] = ObjectIdGetDatum(plpoid); - values[Anum_pg_propgraph_label_property_plppropid - 1] = ObjectIdGetDatum(propoid); - values[Anum_pg_propgraph_label_property_plpellabelid - 1] = ObjectIdGetDatum(ellabeloid); - values[Anum_pg_propgraph_label_property_plpexpr - 1] = CStringGetTextDatum(nodeToString(expr)); - - tup = heap_form_tuple(RelationGetDescr(rel), values, nulls); - CatalogTupleInsert(rel, tup); - heap_freetuple(tup); - - ObjectAddressSet(myself, PropgraphLabelPropertyRelationId, plpoid); - - ObjectAddressSet(referenced, PropgraphPropertyRelationId, propoid); - recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); - - ObjectAddressSet(referenced, PropgraphElementLabelRelationId, ellabeloid); - recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); - - recordDependencyOnSingleRelExpr(&myself, (Node *) copyObject(expr), pgerelid, DEPENDENCY_NORMAL, DEPENDENCY_NORMAL, false); - - table_close(rel, NoLock); - } + insert_label_property_record(ellabeloid, propoid, pgerelid, expr); } /* @@ -1357,7 +1464,7 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt) vinfo->aliasname, stmt->pgname->relname), parser_errposition(pstate, vertex->vtable->location)); - peoid = insert_element_record(pgaddress, vinfo); + peoid = add_element(pgaddress, vinfo); CommandCounterIncrement(); check_element_properties(peoid); @@ -1426,7 +1533,7 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt) einfo->aliasname, stmt->pgname->relname), parser_errposition(pstate, edge->etable->location)); - peoid = insert_element_record(pgaddress, einfo); + peoid = add_element(pgaddress, einfo); CommandCounterIncrement(); check_element_properties(peoid); @@ -1474,7 +1581,6 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt) { PropGraphLabelAndProperties *lp = lfirst_node(PropGraphLabelAndProperties, lc); Oid peoid; - Oid pgerelid; Oid ellabeloid; Assert(lp->label); @@ -1484,10 +1590,9 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt) else peoid = get_edge_oid(pstate, pgrelid, stmt->element_alias, -1); - pgerelid = get_element_relid(peoid); - - ellabeloid = insert_label_record(pgrelid, peoid, lp->label); - insert_property_records(pgrelid, ellabeloid, pgerelid, lp->properties); + ellabeloid = add_element_label_properties(pgrelid, peoid, + get_element_relid(peoid), + lp->label, lp->properties); CommandCounterIncrement(); check_element_properties(peoid); @@ -1610,7 +1715,7 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt) pgerelid = get_element_relid(peoid); - insert_property_records(pgrelid, ellabeloid, pgerelid, stmt->add_properties); + add_label_properties(pgrelid, ellabeloid, pgerelid, stmt->add_properties); CommandCounterIncrement(); check_element_properties(peoid); 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