8.1 and syntax checking at create time

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:

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:

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

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 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.

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 etc
DROP 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

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

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

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

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 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 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

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

When I run this I get this error in the database:
PostgreSQL Error Code: (1)
ERROR: function "plpgsql_validator" does not exist

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

[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

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)