Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 5b56264

Browse filesBrowse files
tglsfdctglsfdc
committed
Teach ANALYZE to clear pg_class.relhassubclass when appropriate.
In the past, relhassubclass always remained true if a relation had ever had child relations, even if the last subclass was long gone. While this had only marginal performance implications in most cases, it was annoying, and I'm now considering some planner changes that would raise the cost of a false positive. It was previously impractical to fix this because of race condition concerns. However, given the recent change that made tablecmds.c take ShareExclusiveLock on relations that are gaining a child (commit fbcf4b9), we can now allow ANALYZE to clear the flag when it's no longer relevant. There is no additional locking cost to do so, since ANALYZE takes ShareExclusiveLock anyway.
1 parent 2f72d5d commit 5b56264
Copy full SHA for 5b56264

File tree

Expand file treeCollapse file tree

4 files changed

+32
-17
lines changed
Filter options
Expand file treeCollapse file tree

4 files changed

+32
-17
lines changed

‎src/backend/catalog/pg_inherits.c

Copy file name to clipboardExpand all lines: src/backend/catalog/pg_inherits.c
+4-3Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,11 +228,12 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents)
228228
* In the current implementation, has_subclass returns whether a
229229
* particular class *might* have a subclass. It will not return the
230230
* correct result if a class had a subclass which was later dropped.
231-
* This is because relhassubclass in pg_class is not updated when a
232-
* subclass is dropped, primarily because of concurrency concerns.
231+
* This is because relhassubclass in pg_class is not updated immediately
232+
* when a subclass is dropped, primarily because of concurrency concerns.
233233
*
234234
* Currently has_subclass is only used as an efficiency hack to skip
235-
* unnecessary inheritance searches, so this is OK.
235+
* unnecessary inheritance searches, so this is OK. Note that ANALYZE
236+
* on a childless table will clean up the obsolete relhassubclass flag.
236237
*
237238
* Although this doesn't actually touch pg_inherits, it seems reasonable
238239
* to keep it here since it's normally used with the other routines here.

‎src/backend/commands/analyze.c

Copy file name to clipboardExpand all lines: src/backend/commands/analyze.c
+7-5Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "catalog/pg_inherits_fn.h"
2727
#include "catalog/pg_namespace.h"
2828
#include "commands/dbcommands.h"
29+
#include "commands/tablecmds.h"
2930
#include "commands/vacuum.h"
3031
#include "executor/executor.h"
3132
#include "miscadmin.h"
@@ -1408,14 +1409,15 @@ acquire_inherited_sample_rows(Relation onerel, HeapTuple *rows, int targrows,
14081409
/*
14091410
* Check that there's at least one descendant, else fail. This could
14101411
* happen despite analyze_rel's relhassubclass check, if table once had a
1411-
* child but no longer does.
1412+
* child but no longer does. In that case, we can clear the
1413+
* relhassubclass field so as not to make the same mistake again later.
1414+
* (This is safe because we hold ShareUpdateExclusiveLock.)
14121415
*/
14131416
if (list_length(tableOIDs) < 2)
14141417
{
1415-
/*
1416-
* XXX It would be desirable to clear relhassubclass here, but we
1417-
* don't have adequate lock to do that safely.
1418-
*/
1418+
/* CCI because we already updated the pg_class row in this command */
1419+
CommandCounterIncrement();
1420+
SetRelationHasSubclass(RelationGetRelid(onerel), false);
14191421
return 0;
14201422
}
14211423

‎src/backend/commands/tablecmds.c

Copy file name to clipboardExpand all lines: src/backend/commands/tablecmds.c
+19-9Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,6 @@ static void StoreCatalogInheritance(Oid relationId, List *supers);
253253
static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
254254
int16 seqNumber, Relation inhRelation);
255255
static int findAttrByName(const char *attributeName, List *schema);
256-
static void setRelhassubclassInRelation(Oid relationId, bool relhassubclass);
257256
static void AlterIndexNamespaces(Relation classRel, Relation rel,
258257
Oid oldNspOid, Oid newNspOid);
259258
static void AlterSeqNamespaces(Relation classRel, Relation rel,
@@ -1359,7 +1358,10 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
13591358
* add children to the same parent simultaneously, and that parent has
13601359
* no pre-existing children, then both will attempt to update the
13611360
* parent's relhassubclass field, leading to a "tuple concurrently
1362-
* updated" error.
1361+
* updated" error. Also, this interlocks against a concurrent ANALYZE
1362+
* on the parent table, which might otherwise be attempting to clear
1363+
* the parent's relhassubclass field, if its previous children were
1364+
* recently dropped.
13631365
*/
13641366
relation = heap_openrv(parent, ShareUpdateExclusiveLock);
13651367

@@ -1958,7 +1960,7 @@ StoreCatalogInheritance1(Oid relationId, Oid parentOid,
19581960
/*
19591961
* Mark the parent as having subclasses.
19601962
*/
1961-
setRelhassubclassInRelation(parentOid, true);
1963+
SetRelationHasSubclass(parentOid, true);
19621964
}
19631965

19641966
/*
@@ -1985,21 +1987,29 @@ findAttrByName(const char *attributeName, List *schema)
19851987
return 0;
19861988
}
19871989

1990+
19881991
/*
1989-
* Update a relation's pg_class.relhassubclass entry to the given value
1992+
* SetRelationHasSubclass
1993+
* Set the value of the relation's relhassubclass field in pg_class.
1994+
*
1995+
* NOTE: caller must be holding an appropriate lock on the relation.
1996+
* ShareUpdateExclusiveLock is sufficient.
1997+
*
1998+
* NOTE: an important side-effect of this operation is that an SI invalidation
1999+
* message is sent out to all backends --- including me --- causing plans
2000+
* referencing the relation to be rebuilt with the new list of children.
2001+
* This must happen even if we find that no change is needed in the pg_class
2002+
* row.
19902003
*/
1991-
static void
1992-
setRelhassubclassInRelation(Oid relationId, bool relhassubclass)
2004+
void
2005+
SetRelationHasSubclass(Oid relationId, bool relhassubclass)
19932006
{
19942007
Relation relationRelation;
19952008
HeapTuple tuple;
19962009
Form_pg_class classtuple;
19972010

19982011
/*
19992012
* Fetch a modifiable copy of the tuple, modify it, update pg_class.
2000-
*
2001-
* If the tuple already has the right relhassubclass setting, we don't
2002-
* need to update it, but we still need to issue an SI inval message.
20032013
*/
20042014
relationRelation = heap_open(RelationRelationId, RowExclusiveLock);
20052015
tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relationId));

‎src/include/commands/tablecmds.h

Copy file name to clipboardExpand all lines: src/include/commands/tablecmds.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ extern void CheckTableNotInUse(Relation rel, const char *stmt);
4343

4444
extern void ExecuteTruncate(TruncateStmt *stmt);
4545

46+
extern void SetRelationHasSubclass(Oid relationId, bool relhassubclass);
47+
4648
extern void renameatt(Oid myrelid, RenameStmt *stmt);
4749

4850
extern void RenameRelation(Oid myrelid,

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.