Thread: 8.1 and syntax checking at create time
I saw in the release notes that 8.1 is supposed to do function syntax checking at create rather than run time, but with the first beta this does not seem to work. check function bodies is on by default in the postgresql.conf file. Is there a setting that didn't make it into the conf file? Thanks, Tony
On Wed, Aug 31, 2005 at 11:39:48AM -0500, Tony Caduto wrote: > I saw in the release notes that 8.1 is supposed to do function syntax > checking at create rather than run time, but with the first beta this > does not seem to work. check function bodies is on by default in the > postgresql.conf file. Is there a setting that didn't make it into the > conf file? It works for me; care to submit an test case? -- Alvaro Herrera -- Valdivia, Chile Architect, www.EnterpriseDB.com "We are who we choose to be", sang the goldfinch when the sun is high (Sandman)
here is a case that does not work: CREATE or REPLACE FUNCTION public.test_func9(out firstname varchar,out lastname varchar) RETURNS SETOF pg_catalog.record AS $BODY$ Declare row record56; BEGIN for $0 in select '',description from common.common_groups loop -- firstname = row.description; -- lastname= ''; RETURN NEXT; end loop; END; $BODY$ LANGUAGE 'plpgsql' VOLATILE; notice the for in select, it's for sure wrong, but it raises no errors until I execute the function also note the declaration for row, there is no record56 type, but it raises no errors at create. here is my version string: PostgreSQL 8.1beta1 on i686-pc-linux-gnu, compiled by GCC i686-pc-linux-gnu-gcc (GCC) 3.3.5-20050130 (Gentoo Linux 3.3.5.20050130-r1, ssp-3.3.5.20050130-1, pie-8.7.7.1) Alvaro Herrera wrote: >On Wed, Aug 31, 2005 at 11:39:48AM -0500, Tony Caduto wrote: > > >>I saw in the release notes that 8.1 is supposed to do function syntax >>checking at create rather than run time, but with the first beta this >>does not seem to work. check function bodies is on by default in the >>postgresql.conf file. Is there a setting that didn't make it into the >>conf file? >> >> > >It works for me; care to submit an test case? > > >
Tony Caduto <tony_caduto@amsoftwaredesign.com> writes:
> notice the for in select, it's for sure wrong, but it raises no errors
> until I execute the function
> also note the declaration for row, there is no record56 type, but it
> raises no errors at create.
It's *syntax* checking, not an exhaustive check that the function is OK.
regards, tom lane
Tom, What exactly does it check then? What I pointed out is simple "syntax" checking in other languages. From what I have seen it does not check anything in the body of the function, I can put gibberish in the body as long as it has a begin and end. It does not seem to be doing anything differently than 8.0.x does with function syntax checking at create time, so why even mention it in the release notes? the function below also raises no errors at create, but at run time it does. If I run the below function I get this error: PostgreSQL Error Code: (1) ERROR: type "record44" does not exist From what I read in the release notes I was expecting to see this raised at create time. Users coming from systems like Oracle and M$ SQL server are expecting this stuff to be caught at create not run time. How difficult would it be to have the server just run the function at create time with null for any input params? Of course a user could just do this but it is a annoying second step. CREATE or REPLACE FUNCTION public.test_func9(out firstname varchar,out lastname varchar) RETURNS SETOF pg_catalog.record AS $BODY$ Declare row record44; BEGIN asfdfdfdfafdsfsdfsdf sdf bla bla sdf yada yada s df sd fsd END; $BODY$ LANGUAGE 'plpgsql' VOLATILE; Thanks, Tony Tom Lane wrote: >Tony Caduto <tony_caduto@amsoftwaredesign.com> writes: > > >>notice the for in select, it's for sure wrong, but it raises no errors >>until I execute the function >>also note the declaration for row, there is no record56 type, but it >>raises no errors at create. >> >> > >It's *syntax* checking, not an exhaustive check that the function is OK. > > regards, tom lane > > >
On Wed, 2005-08-31 at 13:13 -0500, Tony Caduto wrote: > the function below also raises no errors at create, but at run time it does. > ... > CREATE or REPLACE FUNCTION public.test_func9(out firstname varchar,out > lastname varchar) > RETURNS SETOF pg_catalog.record AS > $BODY$ > Declare > row record44; > BEGIN > asfdfdfdfafdsfsdfsdf > sdf bla bla > sdf yada yada > s > df > sd > fsd > END; > $BODY$ > LANGUAGE 'plpgsql' VOLATILE; When I execute this CREATE statement I get: ERROR: type "record44" does not exist CONTEXT: compile of PL/pgSQL function "test_func9" near line 2 So, it does seem to be working as advertised. I'm running HEAD as of a few hours ago.
Tony Caduto <tony_caduto@amsoftwaredesign.com> writes:
> It does not seem to be doing anything differently than 8.0.x does with
> function syntax checking at create time, so why even mention it in the
> release notes?
The checking is more extensive than it was in 8.0. For example 8.0
didn't reject this at creation:
regression=# create function bla() returns int as 'begin zit; end' language plpgsql;
ERROR: syntax error at or near "zit" at character 1
QUERY: zit
CONTEXT: SQL statement in PL/PgSQL function "bla" near line 1
LINE 1: zit ^
regression=#
because 8.0 didn't feed any apparent SQL statements down to the main
SQL grammar to see if they were sane according to the main grammar.
But it remains purely *syntactic*. If the code gets through the
grammar then it's accepted. What this boils down to is that we
don't apply any checking that depends on anything outside the
function itself (for example, whether something that is used as
a type name actually exists in pg_type).
> How difficult would it be to have the server just run the function at
> create time with null for any input params?
What happens if the function (intentionally) errors out on null inputs?
Or goes into an infinite loop? (If it's declared STRICT then the
programmer would be quite within his rights not to handle such a case.)
What happens if the function changes the database on the basis of the
bogus call? How much would this actually prove, considering that null
inputs would be likely not to exercise many of the code paths within the
function?
regards, tom lane
Tony Caduto <tony_caduto@amsoftwaredesign.com> writes:
> CREATE or REPLACE FUNCTION public.test_func9(out firstname varchar,out
> lastname varchar)
> RETURNS SETOF pg_catalog.record AS
> $BODY$
> Declare
> row record44;
> BEGIN
> asfdfdfdfafdsfsdfsdf
> sdf bla bla
> sdf yada yada
> s
> df
> sd
> fsd
> END;
> $BODY$
> LANGUAGE 'plpgsql' VOLATILE;
[ looks at that again... ] Wait, are you sure that you are talking to
an 8.1 server? 8.1 will certainly catch the garbage syntax in the
function body, whether or not it notices that the type name is bogus.
regards, tom lane
On Wed, Aug 31, 2005 at 01:13:00PM -0500, Tony Caduto wrote: > From what I have seen it does not check anything in the body of the > function, I can put gibberish in the body as long as it has a begin and end. > > It does not seem to be doing anything differently than 8.0.x does with > function syntax checking at create time, so why even mention it in the > release notes? I see different behavior in 8.1 than in 8.0. Are you *sure* you're connected to an 8.1 system when you're running your tests? Are you using a database that was restored from an earlier version of PostgreSQL? I wonder if you're not getting the lanvalidator function. What's the result of the following query? SELECT lanname, lanplcallfoid, lanplcallfoid::regprocedure, lanvalidator, lanvalidator::regprocedure FROM pg_language; What happens if you create a fresh database and run "createlang plpgsql" in it, and then run your tests? > the function below also raises no errors at create, but at run time it does. With the example you posted I get the following at create time: ERROR: type "record44" does not exist CONTEXT: compile of PL/pgSQL function "test_func9" near line 2 If I change "record44" to "record" then I get the following (again, at create time): ERROR: syntax error at or near "asfdfdfdfafdsfsdfsdf" at character 1 QUERY: asfdfdfdfafdsfsdfsdf sdf bla bla sdf yada yada s df sd fsd END CONTEXT: SQL statement in PL/PgSQL function "test_func9" near line 10 LINE 1: asfdfdfdfafdsfsdfsdf sdf bla bla sdf yada yada s df sd fsd E... ^ > From what I read in the release notes I was expecting to see this > raised at create time. Create-time checking works here. -- Michael Fuhr
Tony, > From what I have seen it does not check anything in the body of the > function, I can put gibberish in the body as long as it has a begin and > end. Nope: stp=# create function bad_stuff ( x boolean ) returns boolean as $x$ stp$# begin stp$# afasdfasdfasdf; stp$# afasdfasdfa; stp$# asdfasfasdf; stp$# end; stp$# $x$ language plpgsql; ERROR: syntax error at or near "afasdfasdfasdf" at character 1 QUERY: afasdfasdfasdf CONTEXT: SQL statement in PL/PgSQL function "bad_stuff" near line 2 ERROR: syntax error at or near "afasdfasdfasdf" at character 1 QUERY: afasdfasdfasdf CONTEXT: SQL statement in PL/PgSQL function "bad_stuff" near line 2 LINE 1: afasdfasdfasdf Are you sure you don't have check_function_bodies = Off? There is a difference between *syntax* errors and *sql* errors. If a table does not exist, we don't want to check for that and bounce the function; possibly the function will only be called in a context where the table does exist. -- --Josh Josh Berkus Aglio Database Solutions San Francisco
Hi, I did restore from a 8.0 dump. here is the output from the query: lanname | lanplcallfoid | lanplcallfoid | lanvalidator | lanvalidator ----------+---------------+------------------------+--------------+------------------------------internal | 0| - | 2246 | fmgr_internal_validator(oid)c | 0 | - | 2247 | fmgr_c_validator(oid)sql | 0 | - | 2248 | fmgr_sql_validator(oid)plperlu | 16392 | plperl_call_handler() | 0 | -plpgsql | 16394 | plpgsql_call_handler()| 0 | - (5 rows) here is my version string: PostgreSQL 8.1beta1 on i686-pc-linux-gnu, compiled by GCC i686-pc-linux-gnu-gcc (GCC) 3.3.5-20050130 (Gentoo Linux 3.3.5.20050130-r1,ssp-3.3.5.20050130-1, pie-8.7.7.1) I am trying my tests on a new database with fresh language install now. How can I get my restored databases to behave the same as a fresh one? Thanks for your help on this. Tony
Matt, > On Wed, 2005-08-31 at 11:59 -0700, Josh Berkus wrote: > > If a table does not exist, we don't want to check for that and bounce > > the function; possibly the function will only be called in a context > > where the table does exist. > > The Pl/pgSQL compiler should be able to dive into SQL statements, hit > the catalog, and bounce a function because of invalid database object > references. Ideally this capability could be turned off on demand. Well, that would certainly be nice to have as an *additional* capability. Patches welcome! -- --Josh Josh Berkus Aglio Database Solutions San Francisco
Matt Miller <mattm@epx.com> writes:
> On Wed, 2005-08-31 at 11:59 -0700, Josh Berkus wrote:
>> If a table does not exist, we don't want to check for that and bounce
>> the function; possibly the function will only be called in a context
>> where the table does exist.
> I am thankful that Oracle's PL/SQL compiler checks these things for me.
> I don't remember the last time I intended to write code that referenced
> something that did not exist in the database.
Almost every day, people try to write stuff like
CREATE TEMP TABLE foo ... ;INSERT INTO foo ... ;etc etcDROP TABLE foo ;
in plpgsql functions. Now I know that that doesn't work very well,
but we should be going in the direction of fixing it to work well,
not installing error checks that are guaranteed to make it fail.
regards, tom lane
On Wed, 2005-08-31 at 11:59 -0700, Josh Berkus wrote: > If a table does not exist, we don't want to check for that and bounce > the function; possibly the function will only be called in a context > where the table does exist. The Pl/pgSQL compiler should be able to dive into SQL statements, hit the catalog, and bounce a function because of invalid database object references. Ideally this capability could be turned off on demand. I am thankful that Oracle's PL/SQL compiler checks these things for me. I don't remember the last time I intended to write code that referenced something that did not exist in the database. I agree,though, that some developers might rely on such a capability in some circumstances.
Michael Fuhr <mike@fuhr.org> writes:
> Are you using a database that was restored from an earlier version
> of PostgreSQL? I wonder if you're not getting the lanvalidator
> function.
Ah-hah, that sounds like a good theory. He'd have had to have carried
the DB forward from 7.4 or before, though, since plpgsql had a validator
in 8.0.
We've had repeated problems with PL languages stemming from the fact
that pg_dump dumps them at a pretty low semantic level. Aside from this
problem with adding a validator, we used to have issues with hardwired
paths to the shared libraries in the CREATE FUNCTION commands. And in
8.1, whether the functions are in "public" or "pg_catalog" is going to
vary across installations depending on whether the language was restored
from a dump or not.
I wonder if we could change the dump representation to abstract out the
knowledge encapsulated in "createlang". I don't suppose this would
work:
\! createlang plpgsql <dbname>
but it'd be nice if the dump didn't know any more about the language
than its name, and didn't mention the implementation functions at all.
regards, tom lane
On Wed, 2005-08-31 at 15:29 -0400, Tom Lane wrote: > Matt Miller <mattm@epx.com> writes: > > I don't remember the last time I intended to write code that referenced > > something that did not exist in the database. > > Almost every day, people try to write stuff like > > CREATE TEMP TABLE foo ... ; > INSERT INTO foo ... ; > etc etc > DROP TABLE foo ; Point taken. PL/SQL requires all DDL to be dynamic SQL. For example: execute immediate 'drop table foo'; The stuff inside the string is pretty-much ignored at compile time. Maybe, then, my idealized PL/pgSQL compiler always allows DDL to reference any object, but DML is checked against the catalog.
On Wed, Aug 31, 2005 at 03:37:21PM -0400, Andrew Dunstan wrote: > Tony Caduto wrote: > >How can I get my restored databases to behave the same as a fresh one? > > Run "createlang plpgsql mydb" before running your restore, and possibly > remove the bits that create them from the dump script, or they might > just fail benignly. In an already-loaded database, I think the following should work: UPDATE pg_language SET lanvalidator = 'plpgsql_validator'::regproc WHERE lanname = 'plpgsql'; I'd recommend wrapping the update in a transaction and making sure only one record was updated before committing. Tom (or anybody else), are there any gotchas with updating pg_language like this? It works for me in simple tests. -- Michael Fuhr
On Wed, Aug 31, 2005 at 07:43:45PM +0000, Matt Miller wrote: > On Wed, 2005-08-31 at 15:29 -0400, Tom Lane wrote: > > Matt Miller <mattm@epx.com> writes: > > > I don't remember the last time I intended to write code that referenced > > > something that did not exist in the database. > > > > Almost every day, people try to write stuff like > > > > CREATE TEMP TABLE foo ... ; > > INSERT INTO foo ... ; > > etc etc > > DROP TABLE foo ; > > Point taken. > > PL/SQL requires all DDL to be dynamic SQL. For example: > > execute immediate 'drop table foo'; BTW, the way you handled this case in DB2 was: CREATE TEMP TABLE foo ...; CREATE FUNCTION blah AS ...; DROP TEMP TABLE foo; This way the object you wanted did exist when you were creating the function. Of course it would be better if plpgsql could just read the DDL and deal with it... but I'd say that doing the CREATE TABLE outside the statement is better than nothing. Actually, I think you only had to do the CREATE TEMP TABLE outside the function creation if the function didn't create the temp table itself. -- Jim C. Nasby, Sr. Engineering Consultant jnasby@pervasive.com Pervasive Software http://pervasive.com 512-569-9461
On Wed, Aug 31, 2005 at 11:59:47AM -0700, Josh Berkus wrote: > There is a difference between *syntax* errors and *sql* errors. If a > table does not exist, we don't want to check for that and bounce the > function; possibly the function will only be called in a context where the > table does exist. It would still be nice to have, with a way to over-ride it, either via an option to CREATE FUNCTION or with some directive to plpgsql itself inside the function body (probably the most useful case since it allows disabling error checking just where it's needed). -- Jim C. Nasby, Sr. Engineering Consultant jnasby@pervasive.com Pervasive Software http://pervasive.com 512-569-9461
Tony Caduto wrote: > Hi, > > I did restore from a 8.0 dump. > [snip] > I am trying my tests on a new database with fresh language install now. > > How can I get my restored databases to behave the same as a fresh one? > > Run "createlang plpgsql mydb" before running your restore, and possibly remove the bits that create them from the dump script, or they might just fail benignly. cheers andrew
I just found out the databases on 8.0 where originally restored from a 7.4 server, so it seems I have never had the lanvalidator function even while running on 8.0 for the last 10 months :-( So how can I update my restored databases, i tried dropping the language, but it wouldn't let me becasuse of dependent objects. Thanks, Tony Are you using a database that was restored from an earlier version >of PostgreSQL? I wonder if you're not getting the lanvalidator >function. What's the result of the following query? > >SELECT lanname, > lanplcallfoid, lanplcallfoid::regprocedure, > lanvalidator, lanvalidator::regprocedure >FROM pg_language; > >What happens if you create a fresh database and run "createlang >plpgsql" in it, and then run your tests? > > >
[Please don't top-post; it destroys the conversational flow. I've moved your comment below what you commented on.] On Wed, Aug 31, 2005 at 03:13:02PM -0500, Tony Caduto wrote: > >In an already-loaded database, I think the following should work: > > > >UPDATE pg_language SET lanvalidator = 'plpgsql_validator'::regproc > >WHERE lanname = 'plpgsql'; > > When I run this I get this error in the database: > PostgreSQL Error Code: (1) > ERROR: function "plpgsql_validator" does not exist Oops...createlang would ordinarily create that function, but since you restored from another database the validator function was never created. Try adding this before the UPDATE (stolen from pg_dump): CREATE FUNCTION pg_catalog.plpgsql_validator(oid) RETURNS void AS '$libdir/plpgsql', 'plpgsql_validator' LANGUAGE c; -- Michael Fuhr
Tom, I successfully updated my database to use the validator function without dropping it using: CREATE FUNCTION "plpgsql_validator" (oid) RETURNS void AS '$libdir/plpgsql' LANGUAGE C; UPDATE pg_language SET lanvalidator = 'plpgsql_validator'::regproc WHERE lanname = 'plpgsql'; The create checking is *much* better now :-) Thanks to everyone for helping me track this down, turned out it had nothing to do with 8.1 but I didn't know that. Sorry about that. Tony >That would not create a dependency from the language to the validator, >but in practice you probably don't care about that. The bigger problem >for Tony is likely to be that plpgsql_validator() doesn't exist as a >function in his database; he'll have to create it (see createlang -e >for a reference) first. > > regards, tom lane > > >
Michael Fuhr <mike@fuhr.org> writes:
> In an already-loaded database, I think the following should work:
> UPDATE pg_language SET lanvalidator = 'plpgsql_validator'::regproc
> WHERE lanname = 'plpgsql';
> Tom (or anybody else), are there any gotchas with updating pg_language
> like this? It works for me in simple tests.
That would not create a dependency from the language to the validator,
but in practice you probably don't care about that. The bigger problem
for Tony is likely to be that plpgsql_validator() doesn't exist as a
function in his database; he'll have to create it (see createlang -e
for a reference) first.
regards, tom lane
When I run this I get this error in the database: PostgreSQL Error Code: (1) ERROR: function "plpgsql_validator" does not exist >In an already-loaded database, I think the following should work: > >UPDATE pg_language SET lanvalidator = 'plpgsql_validator'::regproc >WHERE lanname = 'plpgsql'; > >I'd recommend wrapping the update in a transaction and making sure >only one record was updated before committing. > >Tom (or anybody else), are there any gotchas with updating pg_language >like this? It works for me in simple tests. > > >
I am new to this hacker's job. What I was looking for was to record
the actual disk IO performed for arbituary query plan. I searched
in backend/executor but not sure if that was the right place to
add a tracer. would the /backend/storage be the place that controls
the actual I/O? btw, is there a way to find the definitions of all variables
or functions defined? I tried cscope but it is not good for such a large
framework.
thanks a lot
huaxin zhang wrote: > I am new to this hacker's job. What I was looking for was to record > the actual disk IO performed for arbituary query plan. I searched > in backend/executor but not sure if that was the right place to > add a tracer. would the /backend/storage be the place that controls > the actual I/O? btw, is there a way to find the definitions of all > variables > or functions defined? I tried cscope but it is not good for such a > large > framework. > > What has this to do with syntax checking? Please don't post irrelevant replies. Start a new topic instead. thanks andrew
On Wed, Aug 31, 2005 at 04:08:10PM -0400, huaxin zhang wrote: > I am new to this hacker's job. What I was looking for was to record > the actual disk IO performed for arbituary query plan. I searched > in backend/executor but not sure if that was the right place to > add a tracer. would the /backend/storage be the place that controls > the actual I/O? btw, is there a way to find the definitions of all variables > or functions defined? I tried cscope but it is not good for such a large > framework. > thanks a lot I believe you'd want backend/storage, but I'm just guessing. -- Jim C. Nasby, Sr. Engineering Consultant jnasby@pervasive.com Pervasive Software http://pervasive.com 512-569-9461
On Thu, Sep 01, 2005 at 09:06:29PM -0500, Jim C. Nasby wrote: > On Wed, Aug 31, 2005 at 04:08:10PM -0400, huaxin zhang wrote: > > I am new to this hacker's job. What I was looking for was to record > > the actual disk IO performed for arbituary query plan. I searched > > in backend/executor but not sure if that was the right place to > > add a tracer. would the /backend/storage be the place that controls > > the actual I/O? btw, is there a way to find the definitions of all variables > > or functions defined? I tried cscope but it is not good for such a large > > framework. > > thanks a lot > > I believe you'd want backend/storage, but I'm just guessing. backend/storage/smgr/md.c -- Alvaro Herrera -- Valdivia, Chile Architect, www.EnterpriseDB.com "Sallah, I said NO camels! That's FIVE camels; can't you count?" (Indiana Jones)