NotificationsYou must be signed in to change notification settings
Fork4.9k
Star17.7k

Commit16d69ec

committed

Remove buggy and dead code from CreateTriggerFiringOn

Here we remove some dead code from CreateTriggerFiringOn() which wasattempting to find the relevant child partition index corresponding to thegiven indexOid. As it turned out, thanks to -Wshadow=compatible-local,this code was buggy as the code which was finding the child indexesassigned those to a shadowed variable that directly went out of scope.The code which thought it was looking at the List of child indexes wasalways referencing an empty List.On further investigation, this code is dead. We never callCreateTriggerFiringOn() passing a valid indexOid in a way that thefunction would actually ever execute the code in question. So, for lackof a way to test if a fix actually works, let's just remove the dead codeinstead.As a reminder, if there is ever a need to resurrect this code, an Assert()has been added to remind future feature developers that they might need towrite some code to find the corresponding child index.Reported-by: Justin PryzbyReviewed-by: Justin PryzbyDiscussion:https://postgr.es/m/20220819211824.GX26426@telsasoft.com

1 parent8c06a28 commit16d69ecCopy full SHA for 16d69ec

File tree

1 file changed

-39

lines changed

src/backend/commands
- trigger.c

1 file changed

-39

lines changed

`‎src/backend/commands/trigger.c`

Lines changed: 5 additions & 39 deletions

Original file line number	Diff line number	Diff line change
`@@ -1147,8 +1147,6 @@ CreateTriggerFiringOn(CreateTrigStmt stmt, const char queryString,`
`1147`	`1147`	`if (partition_recurse)`
`1148`	`1148`	`{`
`1149`	`1149`	`PartitionDescpartdesc=RelationGetPartitionDesc(rel, true);`
`1150`		`-List*idxs=NIL;`
`1151`		`-List*childTbls=NIL;`
`1152`	`1150`	`inti;`
`1153`	`1151`	`MemoryContextoldcxt,`
`1154`	`1152`	`perChildCxt;`
`@@ -1158,53 +1156,23 @@ CreateTriggerFiringOn(CreateTrigStmt stmt, const char queryString,`
`1158`	`1156`	`ALLOCSET_SMALL_SIZES);`
`1159`	`1157`
`1160`	`1158`	`/*`
`1161`		`- *When a trigger is being created associated withan index, we'll`
`1162`		`- *need to associate the trigger in each child partition with the`
`1163`		`- * correspondingindex on it.`
	`1159`	`+ *We don't currently expect to be called witha valid indexOid. If`
	`1160`	`+ *that ever changes then we'll need to quite code here to find the`
	`1161`	`+ * correspondingchild index.`
`1164`	`1162`	`*/`
`1165`		`-if (OidIsValid(indexOid))`
`1166`		`-{`
`1167`		`-ListCell*l;`
`1168`		`-List*idxs=NIL;`
`1169`		`-`
`1170`		`-idxs=find_inheritance_children(indexOid,ShareRowExclusiveLock);`
`1171`		`-foreach(l,idxs)`
`1172`		`-childTbls=lappend_oid(childTbls,`
`1173`		`-IndexGetRelation(lfirst_oid(l),`
`1174`		`- false));`
`1175`		`-}`
	`1163`	`+Assert(!OidIsValid(indexOid));`
`1176`	`1164`
`1177`	`1165`	`oldcxt=MemoryContextSwitchTo(perChildCxt);`
`1178`	`1166`
`1179`	`1167`	`/* Iterate to create the trigger on each existing partition */`
`1180`	`1168`	`for (i=0;i<partdesc->nparts;i++)`
`1181`	`1169`	`{`
`1182`		`-OidindexOnChild=InvalidOid;`
`1183`		`-ListCell*l;`
`1184`		`-ListCell*l2;`
`1185`	`1170`	`CreateTrigStmt*childStmt;`
`1186`	`1171`	`RelationchildTbl;`
`1187`	`1172`	`Node*qual;`
`1188`	`1173`
`1189`	`1174`	`childTbl=table_open(partdesc->oids[i],ShareRowExclusiveLock);`
`1190`	`1175`
`1191`		`-/* Find which of the child indexes is the one on this partition */`
`1192`		`-if (OidIsValid(indexOid))`
`1193`		`-{`
`1194`		`-forboth(l,idxs,l2,childTbls)`
`1195`		`-{`
`1196`		`-if (lfirst_oid(l2)==partdesc->oids[i])`
`1197`		`-{`
`1198`		`-indexOnChild=lfirst_oid(l);`
`1199`		`-break;`
`1200`		`-}`
`1201`		`-}`
`1202`		`-if (!OidIsValid(indexOnChild))`
`1203`		`-elog(ERROR,"failed to find index matching index \"%s\" in partition \"%s\"",`
`1204`		`-get_rel_name(indexOid),`
`1205`		`-get_rel_name(partdesc->oids[i]));`
`1206`		`-}`
`1207`		`-`
`1208`	`1176`	`/*`
`1209`	`1177`	`* Initialize our fabricated parse node by copying the original`
`1210`	`1178`	`* one, then resetting fields that we pass separately.`
`@@ -1224,7 +1192,7 @@ CreateTriggerFiringOn(CreateTrigStmt stmt, const char queryString,`
`1224`	`1192`
`1225`	`1193`	`CreateTriggerFiringOn(childStmt,queryString,`
`1226`	`1194`	`partdesc->oids[i],refRelOid,`
`1227`		`-InvalidOid,indexOnChild,`
	`1195`	`+InvalidOid,InvalidOid,`
`1228`	`1196`	`funcoid,trigoid,qual,`
`1229`	`1197`	`isInternal, true,trigger_fires_when);`
`1230`	`1198`
`@@ -1235,8 +1203,6 @@ CreateTriggerFiringOn(CreateTrigStmt stmt, const char queryString,`
`1235`	`1203`
`1236`	`1204`	`MemoryContextSwitchTo(oldcxt);`
`1237`	`1205`	`MemoryContextDelete(perChildCxt);`
`1238`		`-list_free(idxs);`
`1239`		`-list_free(childTbls);`
`1240`	`1206`	`}`
`1241`	`1207`
`1242`	`1208`	`/* Keep lock on target rel until end of xact */`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commit16d69ec

File tree

1 file changed

1 file changed

`‎src/backend/commands/trigger.c`

0 commit comments