Hmm ... I think you should rather use a PG_TRY/PG_CATCH block.
Thanks for the suggestion, Alvaro -- I think that's a better way to go. It means we can keep vacuum-specific stuff in vacuum.c, rather than adding to AbortTransaction(). I'll post a revised patch tomorrow.
While we're on the subject, the
set_bool_flag do_something_complex reset_bool_flag
coding pattern is really more fragile than it would initially seem to be. It is basically another variant of resource management, in the same way that manual memory management or explicit reference counting can be tricky to get right. For example, if a function that enables the vacuum hint recursively invokes itself, it is liable to reset the vacuum hint earlier than intended (vacuum_rel() comes close to making this mistake, although it does things right). We could make the vacuum hint a counter rather than a bool (bump the counter on "enable hint", decrement it on "disable hint", and treat "hint > 0" as "enabled"), but that just changes the error case slightly -- if you forget to bump/decrement the counter, you're still in trouble.
Perhaps to make this a bit less error prone we could add an assert/elog to StrategyHintVacuum(), which would raise an error/warning if the hint is enabled when it is already true. We shouldn't warn if the flag is disabled when it is already false, since (a) that is harmless (b) it is legitimate in an exception handler, as you suggested.
-Neil
---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match
