Ok, seems that the API issue is settled, so I'm now looking at the code
actually doing the checking. My first impression is that this is a lot
of code. Can we simplify it?
Since this is deeply integrated into the PL/pgSQL interpreter, I was
expecting that this would run through the normal interpreter, in a
special mode that skips all the actual execution of queries, and
shortcuts all loops and other control statements so that all code is
executed only once. That would mean sprinkling some "if (check_only)"
code into the normal exec_* functions. I'm not sure how invasive that
would be, but it's worth considering. I think you would be able to more
easily catch more errors that way, and the check code would stay better
in sync with the execution code.
Another thought is that check_stmt() and all its subroutines are very
similar to the plpgsql_dumptree() code. Would it make sense to merge
those? You could have an output mode, in addition to the xml and
plain-text formats, that would just dump the whole tree like
plpgsql_dumptree() does.
In prepare_expr(), you use a subtransaction to catch any ERRORs that
happen during parsing the expression. That's a good idea, and I think
many of the check_* functions could be greatly simplified by adopting a
similar approach. Just ereport() any errors you find, and catch them at
the appropriate level, appending the error to the output string. Your
current approach of returning true/false depending on whether there was
any errors seems tedious.
If you create a function with an invalid body (ie. set
check_function_bodies=off; create function ... $$ bogus $$;) ,
plpgsql_check_function() still throws an error. It's understandable that
it cannot do in-depth analysis if the function cannot be parsed, but I
would expect the syntax error to be returned as a return value like
other errors that it complains about, not thrown as a hard ERROR. That
would make it more useful to bulk-check all functions in a database with
something like "select plpgsql_check_function(oid) from pg_class". As it
is, the checking stops at the first invalid function with an error.
PS. I think plpgsql_check_function() belongs in pl_handler.c
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
