Acquire ACCESS EXCLUSIVE lock on repack_drop() when trigger exists

[za-arthur]

repack_swap() already drops the trigger repack_trigger and repack_drop() doesn't need to drop it if repack_swap() finished successfully. If repack_drop() doesn't need to drop the trigger it also doesn't need to acquire ACCESS EXCLUSIVE lock which might be beneficial on write heavy tables.

Relevant issue: #467

[Melkij]

I didn't even know we were trying to remove the trigger twice...
It would be nicer to move drop trigger higher, to the lock table itself, but then the numobj calculation would be broken... Do we need this numobj actually? There's an implicit dependency on the order in which temporary objects are created, but after the introduction of drop if exists, it's not really necessary.

[za-arthur]

It would be nicer to move drop trigger higher, to the lock table itself, but then the numobj calculation would be broken... Do we need this numobj actually? There's an implicit dependency on the order in which temporary objects are created, but after the introduction of drop if exists, it's not really necessary.

Yeah, I'm not sure what was the purpose of numobj to be honest. I'd prefer to not have it at all too, I doubt that any user relies on that argument and call it manually.
I'd still leave dropping of the trigger within repack_drop.

[Melkij]

I thought that the counter was needed for older versions of postgresql, when there was no IF EXISTS expression yet. But it turns out the counter isn't that old. It doesn't seem particularly necessary, but it's really not the subject of this PR.

LGTM

[za-arthur]

From the commit message:

This patch includes one global counter (temp_obj_num) which counts number of temporary objects created by pg_repack. Correct order of deletion of temporary object as per count avoids unintentional error messages.

It seems the motivation is to avoid error messages. AFAIK IF EXISTS will log just NOTICE messages.

it's really not the subject of this PR.

+1