diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/backend/catalog/pg_inherits.c | 7 | ||||
| -rw-r--r-- | src/backend/commands/analyze.c | 12 | ||||
| -rw-r--r-- | src/backend/commands/tablecmds.c | 28 | ||||
| -rw-r--r-- | src/include/commands/tablecmds.h | 2 |
4 files changed, 32 insertions, 17 deletions
diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c index ed275d85aad..47290930ea0 100644 --- a/src/backend/catalog/pg_inherits.c +++ b/src/backend/catalog/pg_inherits.c @@ -228,11 +228,12 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents) * In the current implementation, has_subclass returns whether a * particular class *might* have a subclass. It will not return the * correct result if a class had a subclass which was later dropped. - * This is because relhassubclass in pg_class is not updated when a - * subclass is dropped, primarily because of concurrency concerns. + * This is because relhassubclass in pg_class is not updated immediately + * when a subclass is dropped, primarily because of concurrency concerns. * * Currently has_subclass is only used as an efficiency hack to skip - * unnecessary inheritance searches, so this is OK. + * unnecessary inheritance searches, so this is OK. Note that ANALYZE + * on a childless table will clean up the obsolete relhassubclass flag. * * Although this doesn't actually touch pg_inherits, it seems reasonable * to keep it here since it's normally used with the other routines here. diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 1d6301bac16..a9ddb2c2807 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -26,6 +26,7 @@ #include "catalog/pg_inherits_fn.h" #include "catalog/pg_namespace.h" #include "commands/dbcommands.h" +#include "commands/tablecmds.h" #include "commands/vacuum.h" #include "executor/executor.h" #include "miscadmin.h" @@ -1408,14 +1409,15 @@ acquire_inherited_sample_rows(Relation onerel, HeapTuple *rows, int targrows, /* * Check that there's at least one descendant, else fail. This could * happen despite analyze_rel's relhassubclass check, if table once had a - * child but no longer does. + * child but no longer does. In that case, we can clear the + * relhassubclass field so as not to make the same mistake again later. + * (This is safe because we hold ShareUpdateExclusiveLock.) */ if (list_length(tableOIDs) < 2) { - /* - * XXX It would be desirable to clear relhassubclass here, but we - * don't have adequate lock to do that safely. - */ + /* CCI because we already updated the pg_class row in this command */ + CommandCounterIncrement(); + SetRelationHasSubclass(RelationGetRelid(onerel), false); return 0; } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 4509cdab900..1e8ad2b6716 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -253,7 +253,6 @@ static void StoreCatalogInheritance(Oid relationId, List *supers); static void StoreCatalogInheritance1(Oid relationId, Oid parentOid, int16 seqNumber, Relation inhRelation); static int findAttrByName(const char *attributeName, List *schema); -static void setRelhassubclassInRelation(Oid relationId, bool relhassubclass); static void AlterIndexNamespaces(Relation classRel, Relation rel, Oid oldNspOid, Oid newNspOid); static void AlterSeqNamespaces(Relation classRel, Relation rel, @@ -1359,7 +1358,10 @@ MergeAttributes(List *schema, List *supers, char relpersistence, * add children to the same parent simultaneously, and that parent has * no pre-existing children, then both will attempt to update the * parent's relhassubclass field, leading to a "tuple concurrently - * updated" error. + * updated" error. Also, this interlocks against a concurrent ANALYZE + * on the parent table, which might otherwise be attempting to clear + * the parent's relhassubclass field, if its previous children were + * recently dropped. */ relation = heap_openrv(parent, ShareUpdateExclusiveLock); @@ -1958,7 +1960,7 @@ StoreCatalogInheritance1(Oid relationId, Oid parentOid, /* * Mark the parent as having subclasses. */ - setRelhassubclassInRelation(parentOid, true); + SetRelationHasSubclass(parentOid, true); } /* @@ -1985,11 +1987,22 @@ findAttrByName(const char *attributeName, List *schema) return 0; } + /* - * Update a relation's pg_class.relhassubclass entry to the given value + * SetRelationHasSubclass + * Set the value of the relation's relhassubclass field in pg_class. + * + * NOTE: caller must be holding an appropriate lock on the relation. + * ShareUpdateExclusiveLock is sufficient. + * + * NOTE: an important side-effect of this operation is that an SI invalidation + * message is sent out to all backends --- including me --- causing plans + * referencing the relation to be rebuilt with the new list of children. + * This must happen even if we find that no change is needed in the pg_class + * row. */ -static void -setRelhassubclassInRelation(Oid relationId, bool relhassubclass) +void +SetRelationHasSubclass(Oid relationId, bool relhassubclass) { Relation relationRelation; HeapTuple tuple; @@ -1997,9 +2010,6 @@ setRelhassubclassInRelation(Oid relationId, bool relhassubclass) /* * Fetch a modifiable copy of the tuple, modify it, update pg_class. - * - * If the tuple already has the right relhassubclass setting, we don't - * need to update it, but we still need to issue an SI inval message. */ relationRelation = heap_open(RelationRelationId, RowExclusiveLock); tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relationId)); diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index 0e8bbe0929f..333e30326d5 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -43,6 +43,8 @@ extern void CheckTableNotInUse(Relation rel, const char *stmt); extern void ExecuteTruncate(TruncateStmt *stmt); +extern void SetRelationHasSubclass(Oid relationId, bool relhassubclass); + extern void renameatt(Oid myrelid, RenameStmt *stmt); extern void RenameRelation(Oid myrelid, |