mirrors
/
postgres
mirror of https://github.com/postgres/postgres
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove buggy and dead code from CreateTriggerFiringOn 

Here we remove some dead code from CreateTriggerFiringOn() which was
attempting to find the relevant child partition index corresponding to the
given indexOid.  As it turned out, thanks to -Wshadow=compatible-local,
this code was buggy as the code which was finding the child indexes
assigned those to a shadowed variable that directly went out of scope.
The code which thought it was looking at the List of child indexes was
always referencing an empty List.

On further investigation, this code is dead.  We never call
CreateTriggerFiringOn() passing a valid indexOid in a way that the
function would actually ever execute the code in question.  So, for lack
of a way to test if a fix actually works, let's just remove the dead code
instead.

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 to
write some code to find the corresponding child index.

Reported-by: Justin Pryzby
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/20220819211824.GX26426@telsasoft.com
This commit is contained in:
David Rowley 2022-09-06 15:51:44 +12:00
parent 8c06a282fe
commit 16d69ec29b
1 changed files with 5 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
src/backend/commands/trigger.c
View File

@ -1147,8 +1147,6 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
if (partition_recurse)
{
PartitionDesc partdesc = RelationGetPartitionDesc(rel, true);
List *idxs = NIL;
List *childTbls = NIL;
int i;
MemoryContext oldcxt,
perChildCxt;
@ -1158,53 +1156,23 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
ALLOCSET_SMALL_SIZES);
/*
* When a trigger is being created associated with an index, we'll
* need to associate the trigger in each child partition with the
* corresponding index on it.
* We don't currently expect to be called with a valid indexOid. If
* that ever changes then we'll need to quite code here to find the
* corresponding child index.
*/
if (OidIsValid(indexOid))
{
ListCell *l;
List *idxs = NIL;
idxs = find_inheritance_children(indexOid, ShareRowExclusiveLock);
foreach(l, idxs)
childTbls = lappend_oid(childTbls,
IndexGetRelation(lfirst_oid(l),
false));
}
Assert(!OidIsValid(indexOid));
oldcxt = MemoryContextSwitchTo(perChildCxt);
/* Iterate to create the trigger on each existing partition */
for (i = 0; i < partdesc->nparts; i++)
{
Oid indexOnChild = InvalidOid;
ListCell *l;
ListCell *l2;
CreateTrigStmt *childStmt;
Relation childTbl;
Node *qual;
childTbl = table_open(partdesc->oids[i], ShareRowExclusiveLock);
/* Find which of the child indexes is the one on this partition */
if (OidIsValid(indexOid))
{
forboth(l, idxs, l2, childTbls)
{
if (lfirst_oid(l2) == partdesc->oids[i])
{
indexOnChild = lfirst_oid(l);
break;
}
}
if (!OidIsValid(indexOnChild))
elog(ERROR, "failed to find index matching index \"%s\" in partition \"%s\"",
get_rel_name(indexOid),
get_rel_name(partdesc->oids[i]));
}
/*
* Initialize our fabricated parse node by copying the original
* one, then resetting fields that we pass separately.
@ -1224,7 +1192,7 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
CreateTriggerFiringOn(childStmt, queryString,
partdesc->oids[i], refRelOid,
InvalidOid, indexOnChild,
InvalidOid, InvalidOid,
funcoid, trigoid, qual,
isInternal, true, trigger_fires_when);
@ -1235,8 +1203,6 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
MemoryContextSwitchTo(oldcxt);
MemoryContextDelete(perChildCxt);
list_free(idxs);
list_free(childTbls);
}
/* Keep lock on target rel until end of xact */