From: | Dave Page <dpage(at)pgadmin(dot)org> |
---|---|
To: | Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com> |
Cc: | pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: [pgAdmin4][Patch]: Functions/Procedures Module |
Date: | 2016-03-11 16:47:48 |
Message-ID: | CA+OCxowz38ek4d-km9+_pMPqbfHasvx24J5neLmsno0327ftTA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
Hi
On Fri, Mar 11, 2016 at 9:34 AM, Khushboo Vashi
<khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
> Hi,
>
> Please find attached patch for the Functions/Procedures Module.
>
> To test this patch, "Unique collection Control and Variable Control Fixes"
> Patch submitted by me needs to be applied first.
Hi,
This is looking pretty good. I did a little bit of cleanup of the SQL
formatting, however I think a little more is needed;
- When creating a function, we might get the following:
=====
CREATE FUNCTION pem.whee(INOUT foo bigint DEFAULT 123) RETURNS bigint AS
$BODY$SELECT $1;$BODY$
LANGUAGE 'sql' NOT LEAKPROOF
SET backslash_quote='on';
ALTER FUNCTION pem.whee(INOUT foo bigint )
OWNER TO postgres;
GRANT ALL ON FUNCTION pem.whee(INOUT foo bigint ) TO pem_user;
COMMENT ON FUNCTION pem.whee(INOUT foo bigint )
IS 'whee func';
=====
- Remove the spaces after the argument lists, before the )
- Remove the double blank lines
- The opening $BODY$ should be indented
- LANGUAGE should be indented
- The are two spaces between 'sql' and NOT.
- Reverse engineered SQL may look like:
=====
-- FUNCTION: pem.whee(INOUT foo bigint)
-- DROP FUNCTION pem.whee(INOUT foo bigint);
CREATE OR REPLACE FUNCTION pem.whee(INOUT foo bigint DEFAULT 123)
RETURNS bigint
LANGUAGE sql
SET backslash_quote TO 'on'
AS $function$SELECT $1;$function$
=====
- We should be consistent with our heredoc markers ($BODY$ vs. $function$)
- There's a 1 space indent
- This formatting (bar the issues above) is much nicer that the
CREATE formatting.
I haven't found anything else of note yet, that I haven't fixed in
passing (though I have yet to test with PPAS). Updated patch attached.
Thanks.
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachment | Content-Type | Size |
---|---|---|
pgadmin4_functions-dave.patch | application/octet-stream | 226.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Dave Page | 2016-03-11 16:58:31 | pgAdmin 4 commit: Default owner for new event triggers. |
Previous Message | Dave Page | 2016-03-11 16:14:05 | Re: PATCH: Added Node Type & Catalog objects [pgAdmin4] |