Lists: | pgsql-hackers |
---|
From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | patch: CHECK FUNCTION statement |
Date: | 2011-10-06 11:52:07 |
Message-ID: | CAFj8pRDS+A-iR_0k+gDhck2nMZ5iwL5nXPiT0uwDNQtcQ-oQgw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Hello
I am sending a version with regress tests and basic documentation
Regards
Pavel Stehule
Attachment | Content-Type | Size |
---|---|---|
check_function.diff | text/x-patch | 77.0 KB |
From: | Thom Brown <thom(at)linux(dot)com> |
---|---|
To: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: patch: CHECK FUNCTION statement |
Date: | 2011-11-14 20:04:26 |
Message-ID: | CAA-aLv7rwBe0xeXVTna6kDpWf=PkQjtXQZUPz-Hzk-Kc0QCZPw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 6 October 2011 12:52, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Thom Brown <thom(at)linux(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: patch: CHECK FUNCTION statement |
Date: | 2011-11-14 20:54:35 |
Message-ID: | CAFj8pRCcWE_ArCW-eU7nrae9fEW8jUA=YB=Y3xP3hzOotODZwA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Hello
2011/11/14 Thom Brown <thom(at)linux(dot)com>:
> On 6 October 2011 12:52, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>
From: | Thom Brown <thom(at)linux(dot)com> |
---|---|
To: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: patch: CHECK FUNCTION statement |
Date: | 2011-11-14 21:10:20 |
Message-ID: | CAA-aLv7gSOoX6Z0D10fq=H79OMdjU=JC4CavNsZYR-meNPMBiQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 14 November 2011 20:54, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> Hello
>
> 2011/11/14 Thom Brown <thom(at)linux(dot)com>:
>> On 6 October 2011 12:52, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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