fmgr_security_definer() setup and teardown asymmetry

Hi hackers,

While reading at fmgr_security_definer() function implementation I noticed
that setup and teardown are not mirror images of each other:
GetUserIdAndSecContext(&save_userid, &save_sec_context);
save_nestlevel = NewGUCNestLevel(); /* if proconfig */
SetUserIdAndSecContext(fcache->userid, ...); /* if prosecdef */
apply proconfig;
fmgr_hook(FHET_START, ...);
... call the function ...
AtEOXact_GUC(true, save_nestlevel); /* if proconfig */
SetUserIdAndSecContext(save_userid, save_sec_context);
fmgr_hook(FHET_END, ...);

There are two asymmetries:
1. FHET_START is called with the function's user and proconfig in effect,
but FHET_END is called after both have been undone. A hook therefore sees
different state at END than it saw at START. Whatever a hook sets up at
START can't be undone at the same level at END, because the surrounding
state has already been unwound.
2. The GUC nest level is opened before the user switch but closed before
the user is restored. So the two scopes overlap instead of one nesting
inside the other.

The other places that switch the user and isolate GUC changes, such as
vacuum_rel(), do_analyze_rel(), index_build() and cluster_rel(), are
strictly nested:
SetUserIdAndSecContext(owner, ...);
save_nestlevel = NewGUCNestLevel();
...
AtEOXact_GUC(false, save_nestlevel);
SetUserIdAndSecContext(save_userid, save_sec_context);

The attached patch makes fmgr_security_definer() follow the same shape:
save -> switch user -> GUC level -> proconfig -> START
... call ...
END -> close GUC level -> restore user

- The user switch now happens before NewGUCNestLevel(). Proconfig is still
applied after the switch, so its behaviour doesn't change.
- FHET_END is now called before anything is undone, so it runs in the same
state as FHET_START.
- The error path is unchanged: FHET_ABORT is still called from PG_CATCH,
and (sub)transaction abort still cleans up.

Most commonly used fmgr_hook implementations (sepgsql, pg_hint_plan) only
maintain their own state and don't look at the user or GUCs at END, so
they're unaffected.

Thoughts?
--
Alexander Kukushkin