Thread: pgsql-server/ oc/src/sgml/runtime.sgml rc/back ...

pgsql-server/ oc/src/sgml/runtime.sgml rc/back ...

From
tgl@svr1.postgresql.org (Tom Lane)
Date:
CVSROOT:    /cvsroot
Module name:    pgsql-server
Changes by:    tgl@svr1.postgresql.org    03/10/03 16:26:49

Modified files:
    doc/src/sgml   : runtime.sgml
    src/backend/catalog: pg_proc.c
    src/backend/utils/misc: guc.c postgresql.conf.sample

Log message:
    Add GUC parameter check_function_bodies to control whether validation
    of function bodies is done at CREATE FUNCTION time.  This is normally
    true but can be set false to avoid problems with forward references,
    wrong schema search path, etc.  This is just the backend patch, still
    need to adjust pg_dump to make use of it.


Re: pgsql-server/ oc/src/sgml/runtime.sgml rc/back ...

From
Peter Eisentraut
Date:
Tom Lane writes:

>     Add GUC parameter check_function_bodies to control whether validation
>     of function bodies is done at CREATE FUNCTION time.  This is normally
>     true but can be set false to avoid problems with forward references,
>     wrong schema search path, etc.  This is just the backend patch, still
>     need to adjust pg_dump to make use of it.

If it's only honored by SQL functions, then it should probably be called
check_sql_function_bodies.

--
Peter Eisentraut   peter_e@gmx.net


Re: pgsql-server/ oc/src/sgml/runtime.sgml rc/back ...

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane writes:
>> Add GUC parameter check_function_bodies to control whether validation
>> of function bodies is done at CREATE FUNCTION time.

> If it's only honored by SQL functions, then it should probably be called
> check_sql_function_bodies.

I thought about that while I was making the patch, but decided that it
would be a very un-forward-looking name.  Someday we will probably have
syntax-checking validators for plpgsql, etc.

The original version of the patch actually suppressed calling the
validator altogether, but I soon realized that wouldn't do --- it
would allow you to create SQL functions with unsupported pseudotype
arguments or results, for example.  Further thought led me to decide
explicitly not to suspend checking of internal and C function
references, on the grounds that if they are broken they'd still be
broken at the completion of the restore script, and so we'd only
be losing the ability to report the problem.

So the fact that it only affects SQL functions at the moment is IMHO
just happenstance; the scope of what it does will change as we add more
validation capability.

            regards, tom lane

Re: pgsql-server/ oc/src/sgml/runtime.sgml rc/back ...

From
Peter Eisentraut
Date:
Tom Lane writes:

> > If it's only honored by SQL functions, then it should probably be called
> > check_sql_function_bodies.
>
> I thought about that while I was making the patch, but decided that it
> would be a very un-forward-looking name.  Someday we will probably have
> syntax-checking validators for plpgsql, etc.

The point of this feature is to avoid failures because of forward
references in SQL code.  A syntax-checking validator in anything but
possibly plpgsql will not even look at SQL code, so a validator for
a different language will only gain pain and confusion by respecting this
parameter.  Perhaps it needs to different name altogether, along the lines
of "do not check SQL code in functions".

--
Peter Eisentraut   peter_e@gmx.net


Re: pgsql-server/ oc/src/sgml/runtime.sgml rc/back ...

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> The point of this feature is to avoid failures because of forward
> references in SQL code.  A syntax-checking validator in anything but
> possibly plpgsql will not even look at SQL code, so a validator for
> a different language will only gain pain and confusion by respecting this
> parameter.  Perhaps it needs to different name altogether, along the lines
> of "do not check SQL code in functions".

Well, I'm certainly not wedded to the name "check_function_bodies", but
I still feel that you're taking an overly narrow view of the future
functionality of this switch.  The existing reason for having the switch
is not only to avoid forward-reference problems; it also is needed to
avoid search-path problems (eg, if the function assumes "public" is in
the search path, which it won't be during pg_restore).  I think that
issues like these are likely to arise for other sorts of checks than
those on embedded SQL code.  For example, it probably wouldn't be
unreasonable for a validator for brand-X-language to try to check the
existence of referenced functions, even if those functions are called
from code that doesn't look much like SQL.

We could use "check_sql_code" or some such, and it might be an accurate
description of what the switch does today, but I think we'd end up
having to either change the switch name or add more switches of the same
kind in future to cover other checks that turn out to be not always
applicable.  I'd prefer to use a deliberately somewhat vague name to
improve the odds that we don't have to do that.

Would you like it better if the switch were called
do_all_the_right_things_for_pg_dump?  (That name is a bit facetious,
but in terms of long-term behavior that's pretty much what I'm after.)

            regards, tom lane

Re: pgsql-server/ oc/src/sgml/runtime.sgml rc/back ...

From
Peter Eisentraut
Date:
Tom Lane writes:

> I think that issues like these are likely to arise for other sorts of
> checks than those on embedded SQL code.  For example, it probably
> wouldn't be unreasonable for a validator for brand-X-language to try to
> check the existence of referenced functions, even if those functions are
> called from code that doesn't look much like SQL.

Given that new languages don't tend to appear out of the blue, I think
it's reasonable to design the feature considering the languages currently
available.  We have sql, plpgsql, pltcl, plpython, plperl, plruby, plsh,
pljava, maybe something Scheme-based.  None of these languages except the
first two have anything to gain, but everything to lose, if they were
asked not to check the function body during a dump restore.  So do you
have anything more particular in mind?

> Would you like it better if the switch were called
> do_all_the_right_things_for_pg_dump?  (That name is a bit facetious, but
> in terms of long-term behavior that's pretty much what I'm after.)

Would that include altering all sorts of other behaviors, beyond the issue
of function bodies, to facilitate restoring dumps?  That might not be the
worst of ideas, but I'd rather see us improving pg_dump and keep the
relaxed behavior constrained to very well defined areas.

--
Peter Eisentraut   peter_e@gmx.net


Re: pgsql-server/ oc/src/sgml/runtime.sgml rc/back ...

From
Bruce Momjian
Date:
Peter Eisentraut wrote:
> Given that new languages don't tend to appear out of the blue, I think
> it's reasonable to design the feature considering the languages currently
> available.  We have sql, plpgsql, pltcl, plpython, plperl, plruby, plsh,
> pljava, maybe something Scheme-based.  None of these languages except the
> first two have anything to gain, but everything to lose, if they were
> asked not to check the function body during a dump restore.  So do you
> have anything more particular in mind?
>
> > Would you like it better if the switch were called
> > do_all_the_right_things_for_pg_dump?  (That name is a bit facetious, but
> > in terms of long-term behavior that's pretty much what I'm after.)
>
> Would that include altering all sorts of other behaviors, beyond the issue
> of function bodies, to facilitate restoring dumps?  That might not be the
> worst of ideas, but I'd rather see us improving pg_dump and keep the
> relaxed behavior constrained to very well defined areas.

Once we put a GUC value in a dump, we have to keep that parameter valid
almost forever.  I think a general restore GUC setting will be much more
valuable in the future.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: pgsql-server/ oc/src/sgml/runtime.sgml rc/back ...

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Peter Eisentraut wrote:
>> Given that new languages don't tend to appear out of the blue, I think
>> it's reasonable to design the feature considering the languages currently
>> available.

I think that position is sufficiently rebutted by Bruce's observation:

> Once we put a GUC value in a dump, we have to keep that parameter valid
> almost forever.

Since we are inventing this thing specifically to put it in dump files,
we had better take a very long-term view of its purposes.

>> None of these languages except the
>> first two have anything to gain, but everything to lose, if they were
>> asked not to check the function body during a dump restore.

That's why the code leaves it up to the individual validator routine how
much to check or not check depending on the flag setting.  I have no
problem with an individual language deciding that it should or shouldn't
do a particular check.  I do think that we'd be foolish to make advance
judgements about what those decisions will be.

Bottom line is that I wouldn't object to changing the switch name to
something more general ("restore_validation_mode", maybe?) but I think
that changing it to something more specific would be a mistake in the
long run.

            regards, tom lane