Re: [pgadmin4] Edb package support.

From: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgadmin4] Edb package support.
Date: 2016-08-22 12:04:46
Message-ID: CAFiP3vwhGtWTPF=dALsX+7-zyPadj0g-K7wh_mEFkt9AUZby2g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,
PFA patch for wrong header and body were generated.

Fixed the header and body sql parsing issue.

--
*Harshal Dhumal*
*Software Engineer*

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

On Mon, Aug 22, 2016 at 5:10 PM, Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

>
>
> On Mon, Aug 22, 2016 at 3:10 PM, Harshal Dhumal <
> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>
>> 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
>>
>
> This issue is not fixed yet. I am able to reproduce it on PPAS 9.5
>
>
>> 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)or
>>>>> g)
>>>>> > 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
>>>>>
>>>>
>>>>
>>>
>>
>>
>> --
>> 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_sql_parse_issue.patch text/x-patch 2.1 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2016-08-22 12:15:30 Re: [pgadmin4] Edb package support.
Previous Message Khushboo Vashi 2016-08-22 11:40:06 Re: [pgadmin4] Edb package support.