Обсуждение: patch: CHECK FUNCTION statement
Hello I am sending a version with regress tests and basic documentation Regards Pavel Stehule
Вложения
On 6 October 2011 12:52, Pavel Stehule <pavel.stehule@gmail.com> wrote: > > Hello > > I am sending a version with regress tests and basic documentation Hi Pavel, I think this sentence needs rewriting: "checkfunction is the name of a previously registered function that will be called when a new function in the language is created, to check the function by statemnt CHECK FUNCTION or CHECK TRIGGER." to something like: "checkfunction is the name of an existing function that will be called whenever a CHECK FUNCTION or CHECK TRIGGER is requested on a function written in the language." And shouldn't this apply to ALTER LANGUAGE too? And there seem to be copy/paste symptoms in doc/src/sgml/ref/check_function.sgml where it shows the definition of CREATE FUNCTION and CREATE TRIGGER instead of CHECK FUNCTION and CHECK TRIGGER. In src/include/nodes/parsenodes.h there's the error message "there are no plan for query:". This should probably read "there is no plan for query:". This appears more than once. And "cannot to identify real type for record type variable" doesn't sound right. Firstly "to" shouldn't be in there, and referring to a "real" type is ambiguous as there is a data type called "real". This appears at least twice. In src/pl/plpgsql/src/pl_exec.c: "cannot to determine a result of dynamic SQL" should be "cannot determine result of dynamic SQL". Also, I recommend rebasing this patch as it doesn't apply cleanly. In particular, the following fail: src/pl/plpgsql/src/pl_funcs.c src/test/regress/expected/plpgsql.out src/test/regress/sql/plpgsql.sql I haven't tried actually testing the patch itsel, but I will probably give it a go if a rebased version appears. :) -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hello 2011/11/14 Thom Brown <thom@linux.com>: > On 6 October 2011 12:52, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> >> Hello >> >> I am sending a version with regress tests and basic documentation > > Hi Pavel, > > I think this sentence needs rewriting: > > "checkfunction is the name of a previously registered function that > will be called when a new function in the language is created, to > check the function by statemnt CHECK FUNCTION or CHECK TRIGGER." > > to something like: > > "checkfunction is the name of an existing function that will be called > whenever a CHECK FUNCTION or CHECK TRIGGER is requested on a function > written in the language." > > And shouldn't this apply to ALTER LANGUAGE too? > > And there seem to be copy/paste symptoms in > doc/src/sgml/ref/check_function.sgml where it shows the definition of > CREATE FUNCTION and CREATE TRIGGER instead of CHECK FUNCTION and CHECK > TRIGGER. > > In src/include/nodes/parsenodes.h there's the error message "there are > no plan for query:". This should probably read "there is no plan for > query:". This appears more than once. > > And "cannot to identify real type for record type variable" doesn't > sound right. Firstly "to" shouldn't be in there, and referring to a > "real" type is ambiguous as there is a data type called "real". This > appears at least twice. I am not native speaker, so please, fix documentation as you like. > > In src/pl/plpgsql/src/pl_exec.c: > > "cannot to determine a result of dynamic SQL" should be "cannot > determine result of dynamic SQL". > > Also, I recommend rebasing this patch as it doesn't apply cleanly. In > particular, the following fail: > > src/pl/plpgsql/src/pl_funcs.c > src/test/regress/expected/plpgsql.out > src/test/regress/sql/plpgsql.sql > > I haven't tried actually testing the patch itsel, but I will probably > give it a go if a rebased version appears. :) There will be more work, I found one area, that was not checked - expr targets. this new code is on github https://github.com/okbob/plpgsql_lint this week I plan to redesign this contrib module to CHECK FUNCTION implementation for 9.2. Regards Pavel > > -- > Thom Brown > Twitter: @darkixion > IRC (freenode): dark_ixion > Registered Linux user: #516935 > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
On 14 November 2011 20:54, Pavel Stehule <pavel.stehule@gmail.com> wrote: > Hello > > 2011/11/14 Thom Brown <thom@linux.com>: >> On 6 October 2011 12:52, Pavel Stehule <pavel.stehule@gmail.com> wrote: >>> >>> Hello >>> >>> I am sending a version with regress tests and basic documentation >> >> Hi Pavel, >> >> I think this sentence needs rewriting: >> >> "checkfunction is the name of a previously registered function that >> will be called when a new function in the language is created, to >> check the function by statemnt CHECK FUNCTION or CHECK TRIGGER." >> >> to something like: >> >> "checkfunction is the name of an existing function that will be called >> whenever a CHECK FUNCTION or CHECK TRIGGER is requested on a function >> written in the language." >> >> And shouldn't this apply to ALTER LANGUAGE too? >> >> And there seem to be copy/paste symptoms in >> doc/src/sgml/ref/check_function.sgml where it shows the definition of >> CREATE FUNCTION and CREATE TRIGGER instead of CHECK FUNCTION and CHECK >> TRIGGER. >> >> In src/include/nodes/parsenodes.h there's the error message "there are >> no plan for query:". This should probably read "there is no plan for >> query:". This appears more than once. >> >> And "cannot to identify real type for record type variable" doesn't >> sound right. Firstly "to" shouldn't be in there, and referring to a >> "real" type is ambiguous as there is a data type called "real". This >> appears at least twice. > > I am not native speaker, so please, fix documentation as you like. Well I wasn't entirely confident my interpretations were correct. I'd prefer to have a rebased patch I can fully apply first, and then I can provide a corrective patch as I'd like to test it too. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company