2011/11/29 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
> updated patch:
> * recheck compilation and initdb
> * working routines moved to pl_exec.c
> * add entry to catalog.sgml about lanchecker field
> * add node's utils
+ pg_dump support
> Pavel Stehule
> 2011/11/29 Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at>:
>> Pavel Stehule wrote:
>>> I am sending updated patch, that implements a CHECK FUNCTION and CHECK
>>> TRIGGER statements.
>>> This patch is significantly redesigned to previous version (PL/pgSQL
>>> part) - it is more readable, more accurate. There are new regress
>>> Please, can some English native speaker fix doc and comments?
>>> CHECK FUNCTION search function according to function signature - it
>>> should be changes for using a actual types - it can be solution for
>>> polymorphic types and useful tool for work with overloaded functions -
>>> when is not clean, that function was executed.
>>> check function foo(int, int);
>>> NOTICE: checking function foo(variadic anyarray)
>>> and maybe some support for named parameters
>>> check function foo(name text, surname text);
>>> NOTICE: checking function foo(text, text, text, text)
>> I think that CHECK FUNCTION should work exactly like DROP FUNCTION
>> in these respects.
>> Submission review:
>> The patch is context diff, applies with some offsets, contains
>> regression tests and documentation.
>> The documentation should be expanded, the doc for CHECK FUNCTION
>> is only a stub. It should describe the procedure and what is checked.
>> That would also make reviewing easier.
>> I think that some documentation should be added to plhandler.sgml.
>> There is a spelling error (statemnt) in the docs.
>> Usability review:
>> If I understand right, the goal of CHECK FUNCTION is to find errors in
>> the function definition without actually having to execute it.
>> The patch tries to provide this for PL/pgSQL.
>> There hasn't been any discussion on the list, the patch was just posted,
>> so I can't say that we want that. Tom added it to the commitfest page,
>> so there's one important voice against dismissing it right away :^)
>> I don't understand the functional difference between a "validator function"
>> and a "check function" as proposed by this patch. I am probably missing
>> something, but why couldn't these checks be added to function validation
>> when check_function_bodies is set?
>> A new "CHECK FUNCTION" statement could simply call the validator function.
>> I don't see any pg_dump support in this patch, and PL/pgSQL probably doesn't
>> need that, but I think pg_dump support for CREATE LANGUAGE would have to
>> be added for other PLs.
>> I can't test if the functionality is complete because I can't get it to
>> run (see below).
>> Feature test:
>> I can't really test the patch because initdb fails:
>> $ initdb -E UTF8 --locale=de_DE.UTF-8 --lc-messages=en_US.UTF-8 -U postgres /postgres/cvs/dbhome
>> The files belonging to this database system will be owned by user "laurenz".
>> This user must also own the server process.
>> The database cluster will be initialized with locales
>> COLLATE: de_DE.UTF-8
>> CTYPE: de_DE.UTF-8
>> MESSAGES: en_US.UTF-8
>> MONETARY: de_DE.UTF-8
>> NUMERIC: de_DE.UTF-8
>> TIME: de_DE.UTF-8
>> The default text search configuration will be set to "german".
>> creating directory /postgres/cvs/dbhome ... ok
>> creating subdirectories ... ok
>> selecting default max_connections ... 100
>> selecting default shared_buffers ... 32MB
>> creating configuration files ... ok
>> creating template1 database in /postgres/cvs/dbhome/base/1 ... ok
>> initializing pg_authid ... ok
>> initializing dependencies ... ok
>> creating system views ... ok
>> loading system objects' descriptions ... ok
>> creating collations ... ok
>> creating conversions ... ok
>> creating dictionaries ... ok
>> setting privileges on built-in objects ... ok
>> creating information schema ... ok
>> loading PL/pgSQL server-side language ... FATAL: could not load library "/postgres/cvs/pg92/lib/plpgsql.so": /postgres/cvs/pg92/lib/plpgsql.so: undefined symbol: plpgsql_delete_function
>> STATEMENT: CREATE EXTENSION plpgsql;
>> child process exited with exit code 1
>> initdb: removing data directory "/postgres/cvs/dbhome"
>> Coding review:
>> The patch compiles without warnings.
>> The comments in the code should be revised, they are bad English.
>> I can't say if there should be more of them -- I don't know this part of
>> the code well enough to have a well-founded opinion.
>> I don't think there are any portability issues, but I could not test it.
>> There are a lot of small changes to pl/plpgsql/src/pl_exec.c, are they all
>> necessary? For example, why was copy_plpgsql_datum renamed to
>> I'll mark the patch as "Waiting on Author".
>> Laurenz Albe
In response to
pgsql-hackers by date
|Next:||From: Alexander Shulgin||Date: 2011-11-29 19:53:44|
|Subject: Re: Notes on implementing URI syntax for libpq|
|Previous:||From: Pavel Stehule||Date: 2011-11-29 19:37:15|
|Subject: Re: review: CHECK FUNCTION statement|