Re: CREATE FUNCTION .. SET vs. pg_dump

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Sun, Sep 1, 2013 at 10:36 AM, Stefan Kaltenbrunner
> <stefan(at)kaltenbrunner(dot)cc> wrote:
>> attached is a rough patch that does exactly that, if we are worried
>> about an api change we could simple do a ProcessGUCArrayNotice() in the
>> backbranches if that approach is actually sane.

> This patch has some definite coding-style issues, but those should be
> easy to fix. The bigger thing I worry about is whether distributing
> the decision as to what elevel ought to be used here all over the code
> base is indeed sane. Perhaps that ship has already sailed, though.

I don't particularly care for this approach, for that reason and others.
One such issue is that it would result in duplicate messages, because
functioncmds.c will already have issued warnings thanks to the GUCArrayAdd
calls it makes. (Those calls are made with context PGC_S_TEST, which is
why they only throw warnings not errors, cf validate_option_array_item.)

But here's the big-picture consideration: the reason that we call
ProcessGUCArray in this place in ProcedureCreate is that we are trying to
set up the correct GUC environment for the function validator. As an
example, if the SET clause for a SQL-language function includes a setting
for search_path, we had better adopt that search path while checking the
function body, or we will come to incorrect conclusions.

If we use this patch, we'd be saying that we're okay with failing to
apply the SET clause before calling the validator, any time the requested
GUC value happens to be invalid. That's not terribly comfortable: in
general we'd be assuming that validators' results don't depend on the GUC
environment, which is clearly false. Now we can probably get away with
that in the context of check_function_bodies = false, because the
validator shouldn't be doing much of anything then (although a few of them
check some things anyway...). But if we're going to assume that the
validator doesn't depend on GUC environment when check_function_bodies =
false, why apply the SET clause at all?

So I think a more reasonable fix is just to skip the ProcessGUCArray
call altogether when check_function_bodies = false, and to document
that validators mustn't assume anything about the GUC environment
when check_function_bodies = false.

If no objections, I'll go fix it that way.

BTW, I notice the various comments about PGC_S_TEST context are out of
date, because they don't mention that it is used for CREATE FUNCTION SET
cases ...

regards, tom lane