Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> But, when a temp rel is dropped it seems that heap_drop_with_catalog also
>> drops the indexes, so the error occurs when index_drop is called (at least
>> I think this is the case). Certainly commenting out the last two lines
>> *seems* to work.
> Not sure why I introduced that bug in 1.18. Your suggestion was 100%
> correct. I have applied the following patch.
Actually, I don't think this is the true explanation. index_drop'ing
the temp indexes may not be *necessary*, but it shouldn't *hurt* either.
Now that I think about it, the reason for the failure is probably that
there's no CommandCounterIncrement in this loop. Therefore, when we
index_drop an index, the resulting system-table updates are *not seen*
by heap_drop_with_catalog when it comes time to drop the owning table,
and so it tries to drop the index again.
Your solution of not doing index_drop at all is sufficient for the
table-and-index case, but I bet it is not sufficient for more complex
cases like RI checks between temp relations. I'd recommend doing
CommandCounterIncrement after each temp item is dropped, instead.
There is another potential bug here: remove_all_temp_relations() is
failing to consider the possibility that removing one list entry may
cause other ones (eg, indexes) to go away. It's holding onto a "next"
pointer to a list entry that may not be there by the time control comes
back from heap_drop_with_catalog. This is probably OK for tables and
indexes because the indexes will always be added after their table and
thus appear earlier in the list, but again it seems like trouble just
waiting to happen. I would recommend logic along the lines of
while (temp_rels != NIL){ get first entry in temp_rels, and either heap_drop or index_drop it as appropriate;
CommandCounterIncrement();}
This relies on the drop to come back and remove the temp_rels entry
(and, possibly, other entries); so it's not an infinite loop even
though it looks like one.
regards, tom lane
