Re: [pgadmin4] Edb package support.

From: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>
To: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgadmin4] Edb package support.
Date: 2016-08-22 09:40:15
Message-ID: CAFiP3vwxfXm=ZYcyJgifMWt8SYNjfjXA9B691defsiqVFuAK8g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

PFA updated patch for packages

--
*Harshal Dhumal*
*Software Engineer*

EnterpriseDB India: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Aug 18, 2016 at 12:34 PM, Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

> Hi Harshal,
>
> Please see below review comments.
>
> 1. Please make one line space between every SQL block as per pgAdmin 4
> standard.
>
Fixed

> 2. The RE SQL of package is not correct. The package name is displayed
> twice as shown below
>
> CREATE OR REPLACE PACKAGE enterprisedb.empinfo
>
> IS
>
> edb.empinfo is
>
> emp_name character varying(10);
>
> procedure get_name(p_empno numeric);
>
> function display_counter() return integer;
>
> END empinfo;
>
>
> Same applies for CREATE OR REPLACE PACKAGE BODY also.
>
>
Fixed

> 3. The RE SQL of functions, procedures and variable are not correct.
> Currently it is showing the entire package body.
>

Entire package body is stored as free text in single field in database.
There is no way that we can detect sql for particular function/procedure
and therefor I'm showing entire package body. (refer pgadmin3).
And for variables it's showing correct sql (refer pgadmin3)

4. The Create scripts are incorrect for the package and its sub-nodes. If I
> execute that script, it gives me an error.
>
Fixed for package. (fixed in point no. 2)
Create script is not applicable for functions, procedures, variables. I
have disable menu for package child nodes.

> 5. Not able to update the package header as well as body as in the edit
> window the script values are incorrect. So, at the end it will create the
> wrong SQL.
>

Fixed (fixed in point no. 2)

> 6. Please check and change the comments for all the JS files if they are
> copied from another node. Ex: package.js is showing sequence node comments.
>
> Fixed.

> 7. Please maintain 4 tab indentation for SQL templates
>

Fixed

>
> NOTE: I have not repeated Dave's comments, so please incorporate those
> comments in this list.
>
>
> Thanks,
>
> Khushboo
>
>
>
>
>
>
>
>
>
> On Thu, Aug 18, 2016 at 10:50 AM, Khushboo Vashi <
> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>>
>>
>> On Wed, Aug 17, 2016 at 4:35 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Hi,
>>>
>>> I did some quick testing, and found the following issues:
>>>
>>> - Package ACL is not shown in properties
>>>
>> Fxied

>
>>> - Package header and body are not shown in properties
>>>
>>
Fixed

>
>>> - System package? is not shown in properties
>>>
>> Fixed

>
>>> - The header for the RE SQL on a package procedure should read:
>>>
>>> -- Package Procedure
>>>
>>> (not -- Package Function)
>>>
>>
Fixed

>
>>> Can you also have someone (Khushboo?) do a code review, if that hasn't
>>> happened already?
>>>
>>>
>> Sure, will do this.
>>
>>
>>> Thanks.
>>>
>>> On Tue, Aug 16, 2016 at 1:31 PM, Harshal Dhumal
>>> <harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>> > Hi,
>>> >
>>> > PFA initial patch for edb packages.
>>> >
>>> > Other changes:
>>> > 1] Added 'canEdit' flag for node to enable/disabled node edit. (by
>>> default
>>> > it's enable)
>>> > 2] Privileges: Do not show 'ALL' in query if object has only one
>>> applicable
>>> > privilege instead show that privilege.
>>> >
>>> >
>>> > --
>>> > Harshal Dhumal
>>> > Software Engineer
>>> >
>>> > EnterpriseDB India: http://www.enterprisedb.com
>>> > The Enterprise PostgreSQL Company
>>> >
>>> >
>>> > --
>>> > Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org)
>>> > To make changes to your subscription:
>>> > http://www.postgresql.org/mailpref/pgadmin-hackers
>>> >
>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>>
>>> --
>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org)
>>> To make changes to your subscription:
>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>
>>
>>
>

Attachment Content-Type Size
edb_package_support_V2_22_Aug.patch text/x-patch 123.5 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2016-08-22 11:18:40 pgAdmin 4 commit: Allow the use of the tab key to select options in com
Previous Message Surinder Kumar 2016-08-22 07:40:41 Re: [pgAdmin4][Patch]: RM1251 - Tab complete for combo boxes