On Wed, 2023-08-16 at 15:09 -0700, Jeff Davis wrote:
> To bring the overhead closer to zero we need to somehow avoid
> repeating
> so much work in guc.c, though. If we don't go around it, another
> approach would be to separate GUC setting into two phases: one that
> does the checks, and one that performs the final change. All the real
> work (hash lookup, guc_strdup, and check_search_path) is done in the
> "check" phase.
>
> It's a bit painful to reorganize the code in guc.c, though, so that
> might need to happen in a few parts and will depend on how great the
> demand is.
I looked into this, and one problem is that there are two different
memory allocators at work. In guc.c, the new value is guc_strdup'd, and
needs to be saved until it replaces the old value. But if the caller
(in fmgr.c) saves the state in fmgr_security_definer_cache, it would be
saving a guc_strdup'd value, and we'd need to be careful about freeing
that if (and only if) the final set step of the GUC doesn't happen. Or,
we'd need to somehow use pstrdup() instead, and somehow tell guc.c not
to guc_free() it.
Those are obviously solvable, but I don't see a clean way to do so.
Perhaps it's still better than circumventing guc.c entirely (because
that approach probably suffers from the same problem, in addition to
the problem you raised), but I'm not sure if it's worth it to take on
this level of complexity. Suggestions welcome.
Right now I'm still intending to get the cache in place, which doesn't
have these downsides and provides a nice speedup.
Regards,
Jeff Davis