Skip to content

Commit 2a83e03

Browse files
committed
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 cb5c2ba to 9.0 if we wanted to back-patch this.
1 parent 07ab4ec commit 2a83e03

File tree

1 file changed

+56
-18
lines changed

1 file changed

+56
-18
lines changed

src/backend/catalog/dependency.c

Lines changed: 56 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ deleteObjectsInList(ObjectAddresses *targetObjects, Relation *depRel,
244244
* not the direct result of a user-initiated action. For example, when a
245245
* temporary schema is cleaned out so that a new backend can use it, or when
246246
* a column default is dropped as an intermediate step while adding a new one,
247-
* that's an internal operation. On the other hand, when the we drop something
247+
* that's an internal operation. On the other hand, when we drop something
248248
* because the user issued a DROP statement against it, that's not internal.
249249
*/
250250
void
@@ -2102,6 +2102,7 @@ object_address_present_add_flags(const ObjectAddress *object,
21022102
int flags,
21032103
ObjectAddresses *addrs)
21042104
{
2105+
bool result = false;
21052106
int i;
21062107

21072108
for (i = addrs->numrefs - 1; i >= 0; i--)
@@ -2116,22 +2117,48 @@ object_address_present_add_flags(const ObjectAddress *object,
21162117
ObjectAddressExtra *thisextra = addrs->extras + i;
21172118

21182119
thisextra->flags |= flags;
2119-
return true;
2120+
result = true;
21202121
}
2121-
if (thisobj->objectSubId == 0)
2122+
else if (thisobj->objectSubId == 0)
21222123
{
21232124
/*
21242125
* We get here if we find a need to delete a column after
21252126
* having already decided to drop its whole table. Obviously
2126-
* we no longer need to drop the column. But don't plaster
2127-
* its flags on the table.
2127+
* we no longer need to drop the subobject, so report that we
2128+
* found the subobject in the array. But don't plaster its
2129+
* flags on the whole object.
21282130
*/
2129-
return true;
2131+
result = true;
2132+
}
2133+
else if (object->objectSubId == 0)
2134+
{
2135+
/*
2136+
* We get here if we find a need to delete a whole table after
2137+
* having already decided to drop one of its columns. We
2138+
* can't report that the whole object is in the array, but we
2139+
* should mark the subobject with the whole object's flags.
2140+
*
2141+
* It might seem attractive to physically delete the column's
2142+
* array entry, or at least mark it as no longer needing
2143+
* separate deletion. But that could lead to, e.g., dropping
2144+
* the column's datatype before we drop the table, which does
2145+
* not seem like a good idea. This is a very rare situation
2146+
* in practice, so we just take the hit of doing a separate
2147+
* DROP COLUMN action even though we know we're gonna delete
2148+
* the table later.
2149+
*
2150+
* Because there could be other subobjects of this object in
2151+
* the array, this case means we always have to loop through
2152+
* the whole array; we cannot exit early on a match.
2153+
*/
2154+
ObjectAddressExtra *thisextra = addrs->extras + i;
2155+
2156+
thisextra->flags |= flags;
21302157
}
21312158
}
21322159
}
21332160

2134-
return false;
2161+
return result;
21352162
}
21362163

21372164
/*
@@ -2142,6 +2169,7 @@ stack_address_present_add_flags(const ObjectAddress *object,
21422169
int flags,
21432170
ObjectAddressStack *stack)
21442171
{
2172+
bool result = false;
21452173
ObjectAddressStack *stackptr;
21462174

21472175
for (stackptr = stack; stackptr; stackptr = stackptr->next)
@@ -2154,21 +2182,31 @@ stack_address_present_add_flags(const ObjectAddress *object,
21542182
if (object->objectSubId == thisobj->objectSubId)
21552183
{
21562184
stackptr->flags |= flags;
2157-
return true;
2185+
result = true;
2186+
}
2187+
else if (thisobj->objectSubId == 0)
2188+
{
2189+
/*
2190+
* We're visiting a column with whole table already on stack.
2191+
* As in object_address_present_add_flags(), we can skip
2192+
* further processing of the subobject, but we don't want to
2193+
* propagate flags for the subobject to the whole object.
2194+
*/
2195+
result = true;
2196+
}
2197+
else if (object->objectSubId == 0)
2198+
{
2199+
/*
2200+
* We're visiting a table with column already on stack. As in
2201+
* object_address_present_add_flags(), we should propagate
2202+
* flags for the whole object to each of its subobjects.
2203+
*/
2204+
stackptr->flags |= flags;
21582205
}
2159-
2160-
/*
2161-
* Could visit column with whole table already on stack; this is
2162-
* the same case noted in object_address_present_add_flags(), and
2163-
* as in that case, we don't propagate flags for the component to
2164-
* the whole object.
2165-
*/
2166-
if (thisobj->objectSubId == 0)
2167-
return true;
21682206
}
21692207
}
21702208

2171-
return false;
2209+
return result;
21722210
}
21732211

21742212
/*

0 commit comments

Comments
 (0)