Fix dependency searching for case where column is visited before table.

When the recursive search in dependency.c visits a column and then later
visits the whole table containing the column, it needs to propagate the
drop-context flags for the table to the existing target-object entry for
the column.  Otherwise we might refuse the DROP (if not CASCADE) on the
incorrect grounds that there was no automatic drop pathway to the column.
Remarkably, this has not been reported before, though it's possible at
least when an extension creates both a datatype and a table using that
datatype.

Rather than just marking the column as allowed to be dropped, it might
seem good to skip the DROP COLUMN step altogether, since the later DROP
of the table will surely get the job done.  The problem with that is that
the datatype would then be dropped before the table (since the whole
situation occurred because we visited the datatype, and then recursed to
the dependent column, before visiting the table).  That seems pretty risky,
and the case is rare enough that it doesn't seem worth expending a lot of
effort or risk to make the drops happen in a safe order.  So we just play
dumb and delete the column separately according to the existing drop
ordering rules.

Per report from Petr Jelinek, though this is different from his proposed
patch.

Back-patch to 9.1, where extensions were introduced.  There's currently
no evidence that such cases can arise before 9.1, and in any case we would
also need to back-patch cb5c2ba2d8 to 9.0
if we wanted to back-patch this.
This commit is contained in:
Tom Lane 2014-11-11 17:00:11 -05:00
parent 1871c89202
commit 2edfc021c6
1 changed files with 56 additions and 18 deletions

View File

@ -246,7 +246,7 @@ deleteObjectsInList(ObjectAddresses *targetObjects, Relation *depRel,
* not the direct result of a user-initiated action. For example, when a
* temporary schema is cleaned out so that a new backend can use it, or when
* a column default is dropped as an intermediate step while adding a new one,
* that's an internal operation. On the other hand, when the we drop something
* that's an internal operation. On the other hand, when we drop something
* because the user issued a DROP statement against it, that's not internal.
*/
void
@ -2116,6 +2116,7 @@ object_address_present_add_flags(const ObjectAddress *object,
int flags,
ObjectAddresses *addrs)
{
bool result = false;
int i;
for (i = addrs->numrefs - 1; i >= 0; i--)
@ -2130,22 +2131,48 @@ object_address_present_add_flags(const ObjectAddress *object,
ObjectAddressExtra *thisextra = addrs->extras + i;
thisextra->flags |= flags;
return true;
result = true;
}
if (thisobj->objectSubId == 0)
else if (thisobj->objectSubId == 0)
{
/*
* We get here if we find a need to delete a column after
* having already decided to drop its whole table. Obviously
* we no longer need to drop the column. But don't plaster
* its flags on the table.
* we no longer need to drop the subobject, so report that we
* found the subobject in the array. But don't plaster its
* flags on the whole object.
*/
return true;
result = true;
}
else if (object->objectSubId == 0)
{
/*
* We get here if we find a need to delete a whole table after
* having already decided to drop one of its columns. We
* can't report that the whole object is in the array, but we
* should mark the subobject with the whole object's flags.
*
* It might seem attractive to physically delete the column's
* array entry, or at least mark it as no longer needing
* separate deletion. But that could lead to, e.g., dropping
* the column's datatype before we drop the table, which does
* not seem like a good idea. This is a very rare situation
* in practice, so we just take the hit of doing a separate
* DROP COLUMN action even though we know we're gonna delete
* the table later.
*
* Because there could be other subobjects of this object in
* the array, this case means we always have to loop through
* the whole array; we cannot exit early on a match.
*/
ObjectAddressExtra *thisextra = addrs->extras + i;
thisextra->flags |= flags;
}
}
}
return false;
return result;
}
/*
@ -2156,6 +2183,7 @@ stack_address_present_add_flags(const ObjectAddress *object,
int flags,
ObjectAddressStack *stack)
{
bool result = false;
ObjectAddressStack *stackptr;
for (stackptr = stack; stackptr; stackptr = stackptr->next)
@ -2168,21 +2196,31 @@ stack_address_present_add_flags(const ObjectAddress *object,
if (object->objectSubId == thisobj->objectSubId)
{
stackptr->flags |= flags;
return true;
result = true;
}
else if (thisobj->objectSubId == 0)
{
/*
* We're visiting a column with whole table already on stack.
* As in object_address_present_add_flags(), we can skip
* further processing of the subobject, but we don't want to
* propagate flags for the subobject to the whole object.
*/
result = true;
}
else if (object->objectSubId == 0)
{
/*
* We're visiting a table with column already on stack. As in
* object_address_present_add_flags(), we should propagate
* flags for the whole object to each of its subobjects.
*/
stackptr->flags |= flags;
}
/*
* Could visit column with whole table already on stack; this is
* the same case noted in object_address_present_add_flags(), and
* as in that case, we don't propagate flags for the component to
* the whole object.
*/
if (thisobj->objectSubId == 0)
return true;
}
}
return false;
return result;
}
/*