Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commite5fac1c

Browse files
committed
Avoid unnecessary recursion to child tables in ALTER TABLE SET NOT NULL.
If a partitioned table's column is already marked NOT NULL, there isno need to examine its partitions, because we can rely on previousDDL to have enforced that the child columns are NOT NULL as well.(Unfortunately, the same cannot be said for traditional inheritance,so for now we have to restrict the optimization to partitioned tables.)Hence, we may skip recursing to child tables in this situation.The reason this case is worth worrying about is that when pg_dump dumpsa partitioned table having a primary key, it will include the requisiteNOT NULL markings in the CREATE TABLE commands, and then add theprimary key as a separate step. The primary key addition generates aSET NOT NULL as a subcommand, just to be sure. So the situation wherea SET NOT NULL is redundant does arise in the real world.Skipping the recursion does more than just save a few cycles: it meansthat a command such as "ALTER TABLE ONLY partition_parent ADD PRIMARYKEY" will take locks only on the partition parent table, not on thepartitions. It turns out that parallel pg_restore is effectivelyassuming that that's true, and has little choice but to do so becausethe dependencies listed for such a TOC entry don't include thepartitions. pg_restore could thus issue this ALTER while data restoreson the partitions are still in progress. Taking unnecessary locks onthe partitions not only hurts concurrency, but can lead to actualdeadlock failures, as reported by Domagoj Smoljanovic.(A contributing factor in the deadlock is that TRUNCATE on a childpartition wants a non-exclusive lock on the parent. This seemslikewise unnecessary, but the fix for it is more invasive so wewon't consider back-patching it. Fortunately, getting rid of oneof these two poor behaviors is enough to remove the deadlock.)Although support for partitioned primary keys came in with v11,this patch is dependent on the SET NOT NULL refactoring done bycommitf4a3fdf, so we can only patch back to v12.Patch by me; thanks to Alvaro Herrera and Amit Langote for review.Discussion:https://postgr.es/m/VI1PR03MB31670CA1BD9625C3A8C5DD05EB230@VI1PR03MB3167.eurprd03.prod.outlook.com
1 parent3d65b05 commite5fac1c

File tree

1 file changed

+38
-7
lines changed

1 file changed

+38
-7
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5681,14 +5681,10 @@ ATSimpleRecursion(List **wqueue, Relation rel,
56815681
AlterTableUtilityContext *context)
56825682
{
56835683
/*
5684-
* Propagate to children if desired. Only plain tables, foreign tables
5685-
* and partitioned tables have children, so no need to search for other
5686-
* relkinds.
5684+
* Propagate to children, if desired and if there are (or might be) any
5685+
* children.
56875686
*/
5688-
if (recurse &&
5689-
(rel->rd_rel->relkind == RELKIND_RELATION ||
5690-
rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE ||
5691-
rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE))
5687+
if (recurse && rel->rd_rel->relhassubclass)
56925688
{
56935689
Oidrelid = RelationGetRelid(rel);
56945690
ListCell *child;
@@ -6698,6 +6694,41 @@ ATPrepSetNotNull(List **wqueue, Relation rel,
66986694
if (recursing)
66996695
return;
67006696

6697+
/*
6698+
* If the target column is already marked NOT NULL, we can skip recursing
6699+
* to children, because their columns should already be marked NOT NULL as
6700+
* well. But there's no point in checking here unless the relation has
6701+
* some children; else we can just wait till execution to check. (If it
6702+
* does have children, however, this can save taking per-child locks
6703+
* unnecessarily. This greatly improves concurrency in some parallel
6704+
* restore scenarios.)
6705+
*
6706+
* Unfortunately, we can only apply this optimization to partitioned
6707+
* tables, because traditional inheritance doesn't enforce that child
6708+
* columns be NOT NULL when their parent is. (That's a bug that should
6709+
* get fixed someday.)
6710+
*/
6711+
if (rel->rd_rel->relhassubclass &&
6712+
rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
6713+
{
6714+
HeapTupletuple;
6715+
boolattnotnull;
6716+
6717+
tuple = SearchSysCacheAttName(RelationGetRelid(rel), cmd->name);
6718+
6719+
/* Might as well throw the error now, if name is bad */
6720+
if (!HeapTupleIsValid(tuple))
6721+
ereport(ERROR,
6722+
(errcode(ERRCODE_UNDEFINED_COLUMN),
6723+
errmsg("column \"%s\" of relation \"%s\" does not exist",
6724+
cmd->name, RelationGetRelationName(rel))));
6725+
6726+
attnotnull = ((Form_pg_attribute) GETSTRUCT(tuple))->attnotnull;
6727+
ReleaseSysCache(tuple);
6728+
if (attnotnull)
6729+
return;
6730+
}
6731+
67016732
/*
67026733
* If we have ALTER TABLE ONLY ... SET NOT NULL on a partitioned table,
67036734
* apply ALTER TABLE ... CHECK NOT NULL to every child. Otherwise, use

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp