Re: review: CHECK FUNCTION statement - Mailing list pgsql-hackers
From | Albe Laurenz |
---|---|
Subject | Re: review: CHECK FUNCTION statement |
Date | |
Msg-id | D960CB61B694CF459DCFB4B0128514C2072DF93C@exadv11.host.magwien.gv.at Whole thread Raw |
In response to | Re: review: CHECK FUNCTION statement (Pavel Stehule <pavel.stehule@gmail.com>) |
Responses |
Re: review: CHECK FUNCTION statement
Re: review: CHECK FUNCTION statement Re: review: CHECK FUNCTION statement |
List | pgsql-hackers |
Pavel Stehule wrote: > 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 Documentation: -------------- This patch has no documentation for CHECK FUNCTION or CHECK TRIGGER. The last patch had at least something. "\h check function" in psql does not show anything. The patch should also add documentation about the handler function to plhandler.sgml. In particular, the difference between the validator function and the check function should be pointed out. Usability: ---------- Do I understand right that the reason why the check function is different from the validator function is that it would be more difficult to add the checks to the validator function? Is that a good enough argument? From a user's perspective it is difficult to see why some checks are performed at function creation time, while others have to be explicitly checked with CHECK FUNCTION. I think it would be much more intuitive if CHECK FUNCTION does the same as function validation with check_function_bodies on. Submission, Compilation, Regression tests: ------------------------------------------ The patch applies and compiles fine and passes regression tests. The tests cover the functionality. "initdb" succeeds. pg_dump: -------- pg_dump support does not work right. If I create a language like this: CREATE LANGUAGE mylang HANDLER plpgsql_call_handler INLINE plpgsql_inline_handler VALIDATOR plpgsql_validator CHECK plpgsql_checker; the dump will contain: CREATE OR REPLACE PROCEDURAL LANGUAGE mylang; This is not a problem of this patch though (same in 9.1); it seems that pg_dump support for languages without definition in pg_pltemplate is broken in general. Feature test: ------------- CHECK FUNCTION and CHECK TRIGGER work, I couldn't crash it. Error messages could be better: CHECK TRIGGER atrigger; ERROR: syntax error at or near ";" LINE 1: CHECK TRIGGER atrigger; ^ Something like "expected keyword 'ON'" might be nice. There are a lot of things that CHECK FUNCTION does not check, some examples: 1) CREATE OR REPLACE FUNCTION t(i integer) RETURNS integer LANGUAGE plpgsql STRICT AS $$DECLARE j integer; BEGIN IF i=1THEN FOR I IN 1..4 BY -1 LOOP RAISE NOTICE '%', i; END LOOP; RETURN -1; ELSE RETURN2*i; END IF; END;$$; CHECK FUNCTION t(integer); -- no error SELECT t(1); ERROR: BY value of FOR loop must be greater than zero CONTEXT: PL/pgSQL function "t" line 4 at FOR with integer loop variable 2) CREATE OR REPLACE FUNCTION t(i integer) RETURNS integer LANGUAGE plpgsql STRICT AS $$DECLARE j integer; BEGIN IF i=1THEN j=9999999999999999999; RETURN j; ELSE RETURN 2*i; END IF; END;$$; CHECK FUNCTION t(integer); -- no error SELECT t(1); ERROR: value "9999999999999999999" is out of range for type integer CONTEXT: PL/pgSQL function "t" line 4 at assignment 3) CREATE OR REPLACE FUNCTION t(i integer) RETURNS integer LANGUAGE plpgsql STRICT AS $$DECLARE j atable; BEGIN IF i=1 THEN j=12; RETURN j; ELSE RETURN 2*i; END IF; END;$$; CHECK FUNCTION t(integer); -- no error SELECT t(1); ERROR: cannot assign non-composite value to a row variable CONTEXT: PL/pgSQL function "t" line 3 at assignment 4) CREATE TABLE atable( id integer PRIMARY KEY, val text NOT NULL ); INSERT INTO atable VALUES (1, 'eins'); CREATE OR REPLACE FUNCTION atrigger() RETURNS trigger LANGUAGE plpgsql STRICT AS $$BEGIN NEW.id=22; RETURN NEW; END;$$; CREATE TRIGGER atrigger AFTER DELETE ON atable FOR EACH ROW EXECUTE PROCEDURE atrigger(); CHECK TRIGGER atrigger ON atable; -- no error NOTICE: checking function "atrigger()" DELETE FROM atable; ERROR: record "new" is not assigned yet DETAIL: The tuple structure of a not-yet-assigned record is indeterminate. CONTEXT: PL/pgSQL function "atrigger" line 2 at assignment I can try and come up with more if desired. Maybe case 2) and 4) cannot reasonably be covered. It is probably very hard to check everything possible, but the usefulness of CHECK FUNCTION is proportional to the number of cases covered. I'll mark the patch as "Waiting on Author" until there is more documentation, I understand the answers to the questions raised in "usability" above, and until it is agreed that the things checked are sufficient. Yours, Laurenz Albe
pgsql-hackers by date: