From 165d0be4c3938cfb0cc8fa12f212000d2058a14e Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Thu, 27 Aug 2026 21:07:12 +0530 Subject: [PATCH v20260901 4/4] DROP CASCADE to property graph components When dropping objects from a property graph through ALTER PROPERTY GRAPH, it explicitly drops the orphaned labels and properties. But when a DROP CASCADEs to property graph objects, it may leave orphaned labels and properties in the database. This leads to errors when a label with the same name as the orphaned label or a property with the same name as the orphaned property are created with characteristics different from the orphaned one. A pg_propgraph_property entry needs to be removed when the last referencing pg_propgraph_label_property entry is deleted. Similarly a pg_propgraph_label entry needs to be removed when the last referencing pg_propgraph_element_label entry is deleted. Right now there is no way to express a dependency which depends upon the reference count in pg_depend. Ideally we should add invent a new dependency for this and let the dependency tracking infrastructure drop the orphaned properties and labels. But doing so nearer to the release is not possible and the mechanism needs to be well thought through. Instead we modify performDeletion() and performMultipleDeletions() to collect the pg_propgraph_property and pg_propgraph_label entries from the property graph objects being dropped. We use this collection to find out the entries that will be orphaned and delete them. While doing so we take the lock on their parent property graphs so as to avoid inconsistencies being caused by a concurrent ALTER PROPERTY GRAPH. In order to avoid code duplication both AlterPropGraph() and perform*Deletion() code use the same function to remove orphaned objects. Before this change, AlterPropGraph() collected and removed all orphaned entries only ones at the end of the command. Now it would do this for every deletion. Given that ALTER PROPERTY GRAPH can drop only one parent object at a time, the probability of that leading to multiple deletions cascading to a single orphaned object is low. So there's net performance gain with the new approach. When a DROP cascades to any property graph component invalidate caches that reference the parent property graph. AlterPropGraph() does this explicitly but it is missing from DROP CASCADE path. Reported-by: Andres Freund Author: Ashutosh Bapat Reviewed-by: TBD Discussion: https://postgr.es/m/dqa5mstx5mna3i7s23pdwl4m6bek7gqsgfccef44wjpswizufi@3aa6vzri3cat Backpatch-through: 19 --- src/backend/catalog/dependency.c | 291 +++++++++++++++++- src/backend/commands/propgraphcmds.c | 166 +++++----- src/include/commands/propgraphcmds.h | 1 + .../expected/alter-propgraph-drop-cascade.out | 99 ++++++ src/test/isolation/isolation_schedule | 1 + .../specs/alter-propgraph-drop-cascade.spec | 53 ++++ .../expected/create_property_graph.out | 22 ++ src/test/regress/expected/graph_table.out | 34 +- .../regress/sql/create_property_graph.sql | 17 + src/test/regress/sql/graph_table.sql | 21 ++ 10 files changed, 611 insertions(+), 94 deletions(-) create mode 100644 src/test/isolation/expected/alter-propgraph-drop-cascade.out create mode 100644 src/test/isolation/specs/alter-propgraph-drop-cascade.spec diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index eaa524dcbc9..ad3f9a92382 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -76,6 +76,7 @@ #include "commands/event_trigger.h" #include "commands/extension.h" #include "commands/policy.h" +#include "commands/propgraphcmds.h" #include "commands/publicationcmds.h" #include "commands/seclabel.h" #include "commands/sequence.h" @@ -88,6 +89,7 @@ #include "rewrite/rewriteRemove.h" #include "storage/lmgr.h" #include "utils/fmgroids.h" +#include "utils/inval.h" #include "utils/lsyscache.h" #include "utils/syscache.h" @@ -159,7 +161,7 @@ static void findDependentObjects(const ObjectAddress *object, static void performDeletionInternal(ObjectAddresses *targetObjects, DropBehavior behavior, int flags, const ObjectAddress *origObject, - Relation depRel); + Relation *depRel); static void reportDependentObjects(const ObjectAddresses *targetObjects, DropBehavior behavior, int flags, @@ -186,6 +188,22 @@ static bool stack_address_present_add_flags(const ObjectAddress *object, ObjectAddressStack *stack); static void DeleteInitPrivs(const ObjectAddress *object); +/* Property graph related deletion closure routines. */ +static void collectPropGraphCleanupCandidates(const ObjectAddresses *targetObjects, + Relation depRel, + List **labeloids, + List **propoids); +static bool isOrphanedPropGraphObject(const ObjectAddress *object, + Oid associationClassId, + const ObjectAddresses *targetObjects, + Relation depRel); +static void addOrphanedPropGraphObject(const ObjectAddress *object, + ObjectAddresses *targetObjects, + int flags, Relation *depRel); +static void addOrphanedPropGraphObjects(ObjectAddresses *targetObjects, + const List *labeloids, + const List *propoids, + int flags, Relation *depRel); /* * Go through the objects given running the final actions on them, and execute @@ -240,6 +258,201 @@ deleteObjectsInList(ObjectAddresses *targetObjects, Relation *depRel, } } +/* + * Collect pg_propgraph_label and pg_propgraph_property entries that may be + * orphaned when deleting property graph component objects in the + * `targetObjects`. Exclude objects already scheduled for deletion, and return + * the remaining candidates in `labeloids` and `propoids` lists. `depRel` is + * already opened pg_depend relation. + * + * pg_propgraph_label entries are referenced by pg_propgraph_element_label + * entries, and pg_propgraph_property entries are referenced by + * pg_propgraph_label_property entries. Each of these association objects has an + * AUTO dependency on the component object, so we can find them in pg_depend. We + * use pg_depend instead of corresponding catalog tables so that it is easier to + * convert this code to use the dependency traversal code in the future. + */ +static void +collectPropGraphCleanupCandidates(const ObjectAddresses *targetObjects, + Relation depRel, List **labeloids, + List **propoids) +{ + for (int i = 0; i < targetObjects->numrefs; i++) + { + const ObjectAddress *object = &targetObjects->refs[i]; + ObjectAddress referenced; + Oid refobjid = InvalidOid; + Oid refclassid; + List **oids; + ScanKeyData key[2]; + SysScanDesc scan; + HeapTuple tup; + + if (object->classId == PropgraphElementLabelRelationId) + { + refclassid = PropgraphLabelRelationId; + oids = labeloids; + } + else if (object->classId == PropgraphLabelPropertyRelationId) + { + refclassid = PropgraphPropertyRelationId; + oids = propoids; + } + else + continue; + + ScanKeyInit(&key[0], + Anum_pg_depend_classid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(object->classId)); + ScanKeyInit(&key[1], + Anum_pg_depend_objid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(object->objectId)); + + scan = systable_beginscan(depRel, DependDependerIndexId, true, + NULL, 2, key); + while (HeapTupleIsValid(tup = systable_getnext(scan))) + { + Form_pg_depend depform = (Form_pg_depend) GETSTRUCT(tup); + + if (depform->refclassid == refclassid && + depform->deptype == DEPENDENCY_AUTO) + { + if (OidIsValid(refobjid)) + elog(ERROR, "multiple references found for %s in catalog \"%s\"", + getObjectDescription(object, false), + get_rel_name(refclassid)); + + refobjid = depform->refobjid; + } + } + systable_endscan(scan); + + /* + * Each pg_propgraph_label_property entry should have one and only one + * pg_propgraph_property entry. Similarly for + * pg_propgraph_element_label entry and pg_propgraph_label entry. + */ + Assert(OidIsValid(refobjid)); + ObjectAddressSet(referenced, refclassid, refobjid); + if (!object_address_present(&referenced, targetObjects)) + *oids = list_append_unique_oid(*oids, refobjid); + } +} + +/* + * Return true if all property graph components referencing the given object are + * already scheduled for deletion. + * + * object: property graph label or property to check + * associationClassId: catalog containing references to the object + * targetObjects: list of objects that are scheduled to be deleted + * depRel: already opened pg_depend relation + */ +static bool +isOrphanedPropGraphObject(const ObjectAddress *object, Oid associationClassId, + const ObjectAddresses *targetObjects, Relation depRel) +{ + ScanKeyData key[2]; + SysScanDesc scan; + HeapTuple tup; + + ScanKeyInit(&key[0], + Anum_pg_depend_refclassid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(object->classId)); + ScanKeyInit(&key[1], + Anum_pg_depend_refobjid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(object->objectId)); + + scan = systable_beginscan(depRel, DependReferenceIndexId, true, + NULL, 2, key); + while (HeapTupleIsValid(tup = systable_getnext(scan))) + { + Form_pg_depend depform = (Form_pg_depend) GETSTRUCT(tup); + ObjectAddress depender; + + if (depform->classid != associationClassId || + depform->deptype != DEPENDENCY_AUTO) + continue; + + ObjectAddressSubSet(depender, depform->classid, depform->objid, + depform->objsubid); + if (!object_address_present(&depender, targetObjects)) + { + systable_endscan(scan); + return false; + } + } + systable_endscan(scan); + + return true; +} + +/* + * Add an orphan and its dependents to the list of objects being deleted. + * + * The entries in pg_propgraph_label and pg_propgraph_property are respectively + * associated with pg_propgraph_element_label and pg_propgraph_label_property + * entries through AUTO dependencies. Merging that flag into an entry previously + * reached through a NORMAL dependency would incorrectly allow a RESTRICT + * deletion, so preserve the original flags while retaining newly added objects. + */ +static void +addOrphanedPropGraphObject(const ObjectAddress *object, + ObjectAddresses *targetObjects, + int flags, Relation *depRel) +{ + int old_numrefs = targetObjects->numrefs; + int *old_flags; + + old_flags = palloc_array(int, old_numrefs); + for (int i = 0; i < old_numrefs; i++) + old_flags[i] = targetObjects->extras[i].flags; + + AcquireDeletionLock(object, 0); + findDependentObjects(object, DEPFLAG_AUTO, flags, NULL, + targetObjects, NULL, depRel); + + for (int i = 0; i < old_numrefs; i++) + targetObjects->extras[i].flags = old_flags[i]; + pfree(old_flags); +} + +/* + * Add orphaned property graph objects and everything depending on them to the + * deletion list. + */ +static void +addOrphanedPropGraphObjects(ObjectAddresses *targetObjects, + const List *labeloids, const List *propoids, + int flags, Relation *depRel) +{ + foreach_oid(labeloid, labeloids) + { + ObjectAddress object; + + ObjectAddressSet(object, PropgraphLabelRelationId, labeloid); + if (isOrphanedPropGraphObject(&object, + PropgraphElementLabelRelationId, + targetObjects, *depRel)) + addOrphanedPropGraphObject(&object, targetObjects, flags, depRel); + } + + foreach_oid(propoid, propoids) + { + ObjectAddress object; + + ObjectAddressSet(object, PropgraphPropertyRelationId, propoid); + if (isOrphanedPropGraphObject(&object, + PropgraphLabelPropertyRelationId, + targetObjects, *depRel)) + addOrphanedPropGraphObject(&object, targetObjects, flags, depRel); + } +} + /* * performDeletion: attempt to drop the specified object. If CASCADE * behavior is specified, also drop any dependent objects (recursively). @@ -312,7 +525,7 @@ performDeletion(const ObjectAddress *object, NULL, /* no pendingObjects */ &depRel); - performDeletionInternal(targetObjects, behavior, flags, object, depRel); + performDeletionInternal(targetObjects, behavior, flags, object, &depRel); /* And clean up */ free_object_addresses(targetObjects); @@ -378,7 +591,7 @@ performMultipleDeletions(const ObjectAddresses *objects, performDeletionInternal(targetObjects, behavior, flags, (objects->numrefs == 1 ? objects->refs : NULL), - depRel); + &depRel); /* And clean up */ free_object_addresses(targetObjects); @@ -387,7 +600,11 @@ performMultipleDeletions(const ObjectAddresses *objects, } /* - * Complete deletion after the dependency closure has been built. + * Complete deletion after the initial dependency closure has been built. + * + * Additionally this function adds property graph component objects that will be + * orphaned by the scheduled deletions and continue expanding the dependency + * closure. * * targetObjects: list of objects that are scheduled to be deleted * behavior: same as performDeletion() @@ -399,8 +616,30 @@ performMultipleDeletions(const ObjectAddresses *objects, static void performDeletionInternal(ObjectAddresses *targetObjects, DropBehavior behavior, int flags, - const ObjectAddress *origObject, Relation depRel) + const ObjectAddress *origObject, Relation *depRel) { + int old_numrefs; + + /* + * Add property graph component objects that will be orphaned by the + * scheduled deletions. Adding an orphan and its dependents may orphan + * further property graph objects, so continue until the deletion closure + * stops growing. + */ + do + { + List *labeloids = NIL; + List *propoids = NIL; + + old_numrefs = targetObjects->numrefs; + collectPropGraphCleanupCandidates(targetObjects, *depRel, + &labeloids, &propoids); + addOrphanedPropGraphObjects(targetObjects, labeloids, propoids, flags, + depRel); + + list_free(labeloids); + list_free(propoids); + } while (targetObjects->numrefs > old_numrefs); /* * Check if deletion is allowed, and report about cascaded deletes. @@ -408,7 +647,7 @@ performDeletionInternal(ObjectAddresses *targetObjects, reportDependentObjects(targetObjects, behavior, flags, origObject); /* do the deed */ - deleteObjectsInList(targetObjects, &depRel, flags); + deleteObjectsInList(targetObjects, depRel, flags); } /* @@ -1475,6 +1714,20 @@ doDeletion(const ObjectAddress *object, int flags) RemovePublicationById(object->objectId); break; + case PropgraphElementRelationId: + case PropgraphElementLabelRelationId: + case PropgraphLabelRelationId: + case PropgraphLabelPropertyRelationId: + case PropgraphPropertyRelationId: + { + Oid graphoid = GetPropGraphForComponent(object); + + if (OidIsValid(graphoid)) + CacheInvalidateRelcacheByRelid(graphoid); + DropObjectById(object); + break; + } + case CastRelationId: case CollationRelationId: case ConversionRelationId: @@ -1484,11 +1737,6 @@ doDeletion(const ObjectAddress *object, int flags) case AccessMethodRelationId: case AccessMethodOperatorRelationId: case AccessMethodProcedureRelationId: - case PropgraphElementRelationId: - case PropgraphElementLabelRelationId: - case PropgraphLabelRelationId: - case PropgraphLabelPropertyRelationId: - case PropgraphPropertyRelationId: case NamespaceRelationId: case TSParserRelationId: case TSDictionaryRelationId: @@ -1544,6 +1792,27 @@ AcquireDeletionLock(const ObjectAddress *object, int flags) else LockRelationOid(object->objectId, AccessExclusiveLock); } + else if (object->classId == PropgraphElementRelationId || + object->classId == PropgraphElementLabelRelationId || + object->classId == PropgraphLabelRelationId || + object->classId == PropgraphLabelPropertyRelationId || + object->classId == PropgraphPropertyRelationId) + { + Oid graphoid; + + /* Match ALTER PROPERTY GRAPH's graph-before-component lock order. */ + graphoid = GetPropGraphForComponent(object); + if (OidIsValid(graphoid)) + LockRelationOid(graphoid, ShareRowExclusiveLock); + + /* + * We do not really need lock property graph components individually. + * They are all protected by the lock on the property graph itself. + * But maintain the same locking discipline as for other objects. + */ + LockDatabaseObject(object->classId, object->objectId, 0, + AccessExclusiveLock); + } else if (IsSharedRelation(object->classId)) LockSharedObject(object->classId, object->objectId, 0, AccessExclusiveLock); diff --git a/src/backend/commands/propgraphcmds.c b/src/backend/commands/propgraphcmds.c index 2bdce331c49..36cd80d6d59 100644 --- a/src/backend/commands/propgraphcmds.c +++ b/src/backend/commands/propgraphcmds.c @@ -96,7 +96,6 @@ static Oid get_element_relid(Oid peid); static List *get_graph_label_ids(Oid graphid); static List *get_label_element_label_ids(Oid labelid); static List *get_element_label_property_names(Oid ellabeloid); -static List *get_graph_property_ids(Oid graphid); /* @@ -318,6 +317,92 @@ CreatePropGraph(ParseState *pstate, const CreatePropGraphStmt *stmt) return pgaddress; } +/* + * Returns the OID of the property graph containing the given property graph component object. + */ +Oid +GetPropGraphForComponent(const ObjectAddress *object) +{ + HeapTuple tuple; + Relation relation; + ScanKeyData key; + SysScanDesc scan; + + switch (object->classId) + { + case PropgraphElementRelationId: + return GetSysCacheOid1(PROPGRAPHELOID, + Anum_pg_propgraph_element_pgepgid, + ObjectIdGetDatum(object->objectId)); + + case PropgraphLabelRelationId: + return GetSysCacheOid1(PROPGRAPHLABELOID, + Anum_pg_propgraph_label_pglpgid, + ObjectIdGetDatum(object->objectId)); + + case PropgraphPropertyRelationId: + return GetSysCacheOid1(PROPGRAPHPROPOID, + Anum_pg_propgraph_property_pgppgid, + ObjectIdGetDatum(object->objectId)); + + case PropgraphElementLabelRelationId: + { + Oid labeloid = InvalidOid; + + relation = table_open(PropgraphElementLabelRelationId, + AccessShareLock); + ScanKeyInit(&key, Anum_pg_propgraph_element_label_oid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(object->objectId)); + scan = systable_beginscan(relation, + PropgraphElementLabelObjectIndexId, + true, NULL, 1, &key); + tuple = systable_getnext(scan); + if (HeapTupleIsValid(tuple)) + labeloid = ((Form_pg_propgraph_element_label) GETSTRUCT(tuple))->pgellabelid; + systable_endscan(scan); + table_close(relation, AccessShareLock); + + if (OidIsValid(labeloid)) + return GetSysCacheOid1(PROPGRAPHLABELOID, + Anum_pg_propgraph_label_pglpgid, + ObjectIdGetDatum(labeloid)); + return InvalidOid; + } + + case PropgraphLabelPropertyRelationId: + { + Oid propoid = InvalidOid; + + relation = table_open(PropgraphLabelPropertyRelationId, + AccessShareLock); + ScanKeyInit(&key, Anum_pg_propgraph_label_property_oid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(object->objectId)); + scan = systable_beginscan(relation, + PropgraphLabelPropertyObjectIndexId, + true, NULL, 1, &key); + tuple = systable_getnext(scan); + if (HeapTupleIsValid(tuple)) + propoid = ((Form_pg_propgraph_label_property) GETSTRUCT(tuple))->plppropid; + systable_endscan(scan); + table_close(relation, AccessShareLock); + + if (OidIsValid(propoid)) + return GetSysCacheOid1(PROPGRAPHPROPOID, + Anum_pg_propgraph_property_pgppgid, + ObjectIdGetDatum(propoid)); + return InvalidOid; + } + + default: + elog(ERROR, "%s is not a property graph component", + getObjectDescription(object, false)); + } + + pg_unreachable(); +} + /* * Process the key clause specified for an element. If key_clause is non-NIL, * then it is a list of column names. Otherwise, the primary key of the @@ -1494,21 +1579,6 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt) performDeletion(&obj, stmt->drop_behavior, 0); } - /* Remove any orphaned pg_propgraph_label entries */ - if (stmt->drop_vertex_tables || stmt->drop_edge_tables) - { - foreach_oid(labeloid, get_graph_label_ids(pgrelid)) - { - if (!get_label_element_label_ids(labeloid)) - { - ObjectAddress obj; - - ObjectAddressSet(obj, PropgraphLabelRelationId, labeloid); - performDeletion(&obj, stmt->drop_behavior, 0); - } - } - } - foreach(lc, stmt->add_labels) { PropGraphLabelAndProperties *lp = lfirst_node(PropGraphLabelAndProperties, lc); @@ -1610,13 +1680,6 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt) ObjectAddressSet(obj, PropgraphElementLabelRelationId, ellabeloid); performDeletion(&obj, stmt->drop_behavior, 0); - - /* Remove any orphaned pg_propgraph_label entries */ - if (!get_label_element_label_ids(labeloid)) - { - ObjectAddressSet(obj, PropgraphLabelRelationId, labeloid); - performDeletion(&obj, stmt->drop_behavior, 0); - } } if (stmt->add_properties) @@ -1714,35 +1777,6 @@ AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt) check_element_label_properties(ellabeloid); } - /* Remove any orphaned pg_propgraph_property entries */ - if (stmt->drop_properties || stmt->drop_vertex_tables || stmt->drop_edge_tables || stmt->drop_label) - { - foreach_oid(propoid, get_graph_property_ids(pgrelid)) - { - Relation rel; - SysScanDesc scan; - ScanKeyData key[1]; - - rel = table_open(PropgraphLabelPropertyRelationId, RowShareLock); - ScanKeyInit(&key[0], - Anum_pg_propgraph_label_property_plppropid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(propoid)); - /* XXX no suitable index */ - scan = systable_beginscan(rel, InvalidOid, true, NULL, 1, key); - if (!systable_getnext(scan)) - { - ObjectAddress obj; - - ObjectAddressSet(obj, PropgraphPropertyRelationId, propoid); - performDeletion(&obj, stmt->drop_behavior, 0); - } - - systable_endscan(scan); - table_close(rel, RowShareLock); - } - } - /* * Invalidate relcache entry of the property graph so that the queries in * the cached plans referencing the property graph will be rewritten @@ -1929,31 +1963,3 @@ get_element_label_property_names(Oid ellabeloid) return result; } - -/* - * Get a list of all property OIDs of a graph. - */ -static List * -get_graph_property_ids(Oid graphid) -{ - Relation rel; - SysScanDesc scan; - ScanKeyData key[1]; - HeapTuple tuple; - List *result = NIL; - - rel = table_open(PropgraphPropertyRelationId, AccessShareLock); - ScanKeyInit(&key[0], - Anum_pg_propgraph_property_pgppgid, - BTEqualStrategyNumber, - F_OIDEQ, ObjectIdGetDatum(graphid)); - scan = systable_beginscan(rel, PropgraphPropertyNameIndexId, true, NULL, 1, key); - while (HeapTupleIsValid(tuple = systable_getnext(scan))) - { - result = lappend_oid(result, ((Form_pg_propgraph_property) GETSTRUCT(tuple))->oid); - } - systable_endscan(scan); - table_close(rel, AccessShareLock); - - return result; -} diff --git a/src/include/commands/propgraphcmds.h b/src/include/commands/propgraphcmds.h index 1bf7d9ea217..f75d54dc016 100644 --- a/src/include/commands/propgraphcmds.h +++ b/src/include/commands/propgraphcmds.h @@ -19,5 +19,6 @@ extern ObjectAddress CreatePropGraph(ParseState *pstate, const CreatePropGraphStmt *stmt); extern ObjectAddress AlterPropGraph(ParseState *pstate, const AlterPropGraphStmt *stmt); +extern Oid GetPropGraphForComponent(const ObjectAddress *object); #endif /* PROPGRAPHCMDS_H */ diff --git a/src/test/isolation/expected/alter-propgraph-drop-cascade.out b/src/test/isolation/expected/alter-propgraph-drop-cascade.out new file mode 100644 index 00000000000..32b99c0c40c --- /dev/null +++ b/src/test/isolation/expected/alter-propgraph-drop-cascade.out @@ -0,0 +1,99 @@ +Parsed test spec with 2 sessions + +starting permutation: s1_begin s1_add_label s2_drop s1_commit s2_check +step s1_begin: BEGIN; +step s1_add_label: + ALTER PROPERTY GRAPH pgg ALTER VERTEX TABLE pgt_keep + ADD LABEL pgl PROPERTIES (a AS p); + +step s2_drop: DROP TABLE pgt_drop CASCADE; +step s1_commit: COMMIT; +s2: NOTICE: drop cascades to vertex pgt_drop of property graph pgg +step s2_drop: <... completed> +step s2_check: + SELECT + (SELECT count(*) + FROM information_schema.pg_labels + WHERE property_graph_name = 'pgg' AND label_name = 'pgl') AS labels, + (SELECT count(*) + FROM information_schema.pg_property_data_types + WHERE property_graph_name = 'pgg' AND property_name = 'p') AS properties; + +labels|properties +------+---------- + 1| 1 +(1 row) + + +starting permutation: s1_begin s1_add_label s2_drop s1_rollback s2_check +step s1_begin: BEGIN; +step s1_add_label: + ALTER PROPERTY GRAPH pgg ALTER VERTEX TABLE pgt_keep + ADD LABEL pgl PROPERTIES (a AS p); + +step s2_drop: DROP TABLE pgt_drop CASCADE; +step s1_rollback: ROLLBACK; +s2: NOTICE: drop cascades to vertex pgt_drop of property graph pgg +step s2_drop: <... completed> +step s2_check: + SELECT + (SELECT count(*) + FROM information_schema.pg_labels + WHERE property_graph_name = 'pgg' AND label_name = 'pgl') AS labels, + (SELECT count(*) + FROM information_schema.pg_property_data_types + WHERE property_graph_name = 'pgg' AND property_name = 'p') AS properties; + +labels|properties +------+---------- + 0| 0 +(1 row) + + +starting permutation: s1_begin s2_drop s1_add_label s1_commit s2_check +step s1_begin: BEGIN; +s2: NOTICE: drop cascades to vertex pgt_drop of property graph pgg +step s2_drop: DROP TABLE pgt_drop CASCADE; +step s1_add_label: + ALTER PROPERTY GRAPH pgg ALTER VERTEX TABLE pgt_keep + ADD LABEL pgl PROPERTIES (a AS p); + +step s1_commit: COMMIT; +step s2_check: + SELECT + (SELECT count(*) + FROM information_schema.pg_labels + WHERE property_graph_name = 'pgg' AND label_name = 'pgl') AS labels, + (SELECT count(*) + FROM information_schema.pg_property_data_types + WHERE property_graph_name = 'pgg' AND property_name = 'p') AS properties; + +labels|properties +------+---------- + 1| 1 +(1 row) + + +starting permutation: s1_begin s2_drop s1_add_label s1_rollback s2_check +step s1_begin: BEGIN; +s2: NOTICE: drop cascades to vertex pgt_drop of property graph pgg +step s2_drop: DROP TABLE pgt_drop CASCADE; +step s1_add_label: + ALTER PROPERTY GRAPH pgg ALTER VERTEX TABLE pgt_keep + ADD LABEL pgl PROPERTIES (a AS p); + +step s1_rollback: ROLLBACK; +step s2_check: + SELECT + (SELECT count(*) + FROM information_schema.pg_labels + WHERE property_graph_name = 'pgg' AND label_name = 'pgl') AS labels, + (SELECT count(*) + FROM information_schema.pg_property_data_types + WHERE property_graph_name = 'pgg' AND property_name = 'p') AS properties; + +labels|properties +------+---------- + 0| 0 +(1 row) + diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 1fcf4e63238..8769a9d3332 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -130,3 +130,4 @@ test: for-portion-of test: ddl-dependency-locking test: pub-concurrent-drop test: drop-owned-grant +test: alter-propgraph-drop-cascade diff --git a/src/test/isolation/specs/alter-propgraph-drop-cascade.spec b/src/test/isolation/specs/alter-propgraph-drop-cascade.spec new file mode 100644 index 00000000000..4f577c79ec8 --- /dev/null +++ b/src/test/isolation/specs/alter-propgraph-drop-cascade.spec @@ -0,0 +1,53 @@ +# Test a cascaded element-table drop concurrent with property graph changes. + +setup +{ + CREATE TABLE pgt_drop (a int PRIMARY KEY); + CREATE TABLE pgt_keep (a int PRIMARY KEY); + CREATE PROPERTY GRAPH pgg + VERTEX TABLES ( + pgt_drop LABEL pgl PROPERTIES (a AS p), + pgt_keep); +} + +teardown +{ + DROP PROPERTY GRAPH pgg; + DROP TABLE IF EXISTS pgt_drop; + DROP TABLE pgt_keep; +} + +session s1 +step s1_begin { BEGIN; } +step s1_add_label +{ + ALTER PROPERTY GRAPH pgg ALTER VERTEX TABLE pgt_keep + ADD LABEL pgl PROPERTIES (a AS p); +} +step s1_commit { COMMIT; } +step s1_rollback { ROLLBACK; } + +session s2 +step s2_drop { DROP TABLE pgt_drop CASCADE; } +step s2_check +{ + SELECT + (SELECT count(*) + FROM information_schema.pg_labels + WHERE property_graph_name = 'pgg' AND label_name = 'pgl') AS labels, + (SELECT count(*) + FROM information_schema.pg_property_data_types + WHERE property_graph_name = 'pgg' AND property_name = 'p') AS properties; +} + +# Committing ADD LABEL should keep the shared label and property. +permutation s1_begin s1_add_label s2_drop s1_commit s2_check + +# Rolling back ADD LABEL should remove the orphaned metadata. +permutation s1_begin s1_add_label s2_drop s1_rollback s2_check + +# ADD LABEL after DROP should recreate the label and property on commit. +permutation s1_begin s2_drop s1_add_label s1_commit s2_check + +# Rolling back ADD LABEL after DROP should leave the metadata removed. +permutation s1_begin s2_drop s1_add_label s1_rollback s2_check diff --git a/src/test/regress/expected/create_property_graph.out b/src/test/regress/expected/create_property_graph.out index 68fe1388b05..fbfa629c578 100644 --- a/src/test/regress/expected/create_property_graph.out +++ b/src/test/regress/expected/create_property_graph.out @@ -118,6 +118,28 @@ ERROR: property graph "g4" element "t2" label "t2" has no property "yy" -- because it remains associated with t3l1. We will verify this in the -- information schema queries outputs below. ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t3 DROP LABEL t3l2; +-- Cascaded drop should clean up the orphaned labels and properties which should +-- be absent from the information schema query outputs below. The objects +-- dependent on the orphaned labels or properties should also be dropped. +CREATE TABLE t_tmp (p_tmp int primary key); +ALTER PROPERTY GRAPH g4 ADD VERTEX TABLES (t_tmp); +ALTER PROPERTY GRAPH g3 ADD VERTEX TABLES (t_tmp); +CREATE FUNCTION orphan_f() RETURNS int LANGUAGE SQL +BEGIN ATOMIC + SELECT p_tmp FROM GRAPH_TABLE ( + g3 MATCH (v IS t_tmp) COLUMNS (v.p_tmp AS p_tmp)); +END; +ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t2 ALTER LABEL t2 + ADD PROPERTIES (orphan_f() AS cascade_orphan); +SET client_min_messages = warning; +DROP TABLE t_tmp CASCADE; +RESET client_min_messages; +\df orphan_f + List of functions + Schema | Name | Result data type | Argument data types | Type +--------+------+------------------+---------------------+------ +(0 rows) + -- edge and vertex table key inference from primary and foreign key constraints. CREATE TABLE t11 (a int PRIMARY KEY); CREATE TABLE t12 (b int PRIMARY KEY); diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index 2e862a82ba0..b1166e5cb8c 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -871,6 +871,34 @@ EXECUTE loopstmt; e331_new (2 rows) +-- A cascaded drop must invalidate a cached plan. If we drop an element table, +-- the plan is invalidated because it depends upon the table, not because a +-- dependent property graph object getting dropped. Instead use an object like +-- collation which when dropped does not cause the plan to be invalidated. +CREATE COLLATION cache_coll FROM "C"; +CREATE TABLE cache_v (id text PRIMARY KEY); +INSERT INTO cache_v VALUES ('one'); +CREATE PROPERTY GRAPH cache_g + VERTEX TABLES (cache_v LABEL cache_l + PROPERTIES (id COLLATE cache_coll AS p)); +PREPARE cachestmt AS + SELECT p + FROM GRAPH_TABLE (cache_g MATCH (v IS cache_l) COLUMNS (v.p AS p)); +EXECUTE cachestmt; + p +----- + one +(1 row) + +DROP COLLATION cache_coll CASCADE; +NOTICE: drop cascades to property p of property graph cache_g +EXECUTE cachestmt; -- error +ERROR: property "p" does not exist +DEALLOCATE cachestmt; +-- Don't retain this property graph for dump/restore testing later since there +-- is nothing special to test in an empty property graph. +DROP PROPERTY GRAPH cache_g; +DROP TABLE cache_v; -- inheritance and partitioning CREATE TABLE pv (id int, val int); CREATE TABLE cv1 () INHERITS (pv); @@ -986,17 +1014,17 @@ SELECT g.* FROM x1, -- If these DDLs succeed, the pg_get_viewdef call below will throw cache lookup -- error. ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE orders DROP LABEL orders; -- error -ERROR: cannot drop label orders of property graph myshop because other objects depend on it +ERROR: cannot drop label orders of vertex orders of property graph myshop because other objects depend on it DETAIL: view customers_us depends on label orders of property graph myshop HINT: Use DROP ... CASCADE to drop the dependent objects too. ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE customers ALTER LABEL customers DROP PROPERTIES (address); -- error -ERROR: cannot drop property address of property graph myshop because other objects depend on it +ERROR: cannot drop property address of label customers of vertex customers of property graph myshop because other objects depend on it DETAIL: view customers_us depends on property address of property graph myshop HINT: Use DROP ... CASCADE to drop the dependent objects too. ALTER PROPERTY GRAPH myshop ALTER VERTEX TABLE products ALTER LABEL products DROP PROPERTIES (price); -- error -ERROR: cannot drop property price of property graph myshop because other objects depend on it +ERROR: cannot drop property price of label products of vertex products of property graph myshop because other objects depend on it DETAIL: view customers_us depends on property price of property graph myshop HINT: Use DROP ... CASCADE to drop the dependent objects too. -- ruleutils reverse parsing diff --git a/src/test/regress/sql/create_property_graph.sql b/src/test/regress/sql/create_property_graph.sql index 17ef9de0ec0..9e79d7d02d9 100644 --- a/src/test/regress/sql/create_property_graph.sql +++ b/src/test/regress/sql/create_property_graph.sql @@ -103,6 +103,23 @@ ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t2 ALTER LABEL t2 DROP PROPERTIES (yy -- because it remains associated with t3l1. We will verify this in the -- information schema queries outputs below. ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t3 DROP LABEL t3l2; +-- Cascaded drop should clean up the orphaned labels and properties which should +-- be absent from the information schema query outputs below. The objects +-- dependent on the orphaned labels or properties should also be dropped. +CREATE TABLE t_tmp (p_tmp int primary key); +ALTER PROPERTY GRAPH g4 ADD VERTEX TABLES (t_tmp); +ALTER PROPERTY GRAPH g3 ADD VERTEX TABLES (t_tmp); +CREATE FUNCTION orphan_f() RETURNS int LANGUAGE SQL +BEGIN ATOMIC + SELECT p_tmp FROM GRAPH_TABLE ( + g3 MATCH (v IS t_tmp) COLUMNS (v.p_tmp AS p_tmp)); +END; +ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t2 ALTER LABEL t2 + ADD PROPERTIES (orphan_f() AS cascade_orphan); +SET client_min_messages = warning; +DROP TABLE t_tmp CASCADE; +RESET client_min_messages; +\df orphan_f -- edge and vertex table key inference from primary and foreign key constraints. CREATE TABLE t11 (a int PRIMARY KEY); diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index 21e70015f6e..eb752ca2725 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -481,6 +481,27 @@ ALTER PROPERTY GRAPH g1 ALTER EDGE TABLE e3_3 ALTER LABEL l2 DROP PROPERTIES (el EXECUTE loopstmt; -- error ALTER PROPERTY GRAPH g1 ALTER EDGE TABLE e3_3 ALTER LABEL l2 ADD PROPERTIES ((ename || '_new')::varchar(10) AS elname); EXECUTE loopstmt; +-- A cascaded drop must invalidate a cached plan. If we drop an element table, +-- the plan is invalidated because it depends upon the table, not because a +-- dependent property graph object getting dropped. Instead use an object like +-- collation which when dropped does not cause the plan to be invalidated. +CREATE COLLATION cache_coll FROM "C"; +CREATE TABLE cache_v (id text PRIMARY KEY); +INSERT INTO cache_v VALUES ('one'); +CREATE PROPERTY GRAPH cache_g + VERTEX TABLES (cache_v LABEL cache_l + PROPERTIES (id COLLATE cache_coll AS p)); +PREPARE cachestmt AS + SELECT p + FROM GRAPH_TABLE (cache_g MATCH (v IS cache_l) COLUMNS (v.p AS p)); +EXECUTE cachestmt; +DROP COLLATION cache_coll CASCADE; +EXECUTE cachestmt; -- error +DEALLOCATE cachestmt; +-- Don't retain this property graph for dump/restore testing later since there +-- is nothing special to test in an empty property graph. +DROP PROPERTY GRAPH cache_g; +DROP TABLE cache_v; -- inheritance and partitioning CREATE TABLE pv (id int, val int); -- 2.34.1