Обсуждение: [pgadmin4] Edb package support.

Hi,

I did some quick testing, and found the following issues:

- Package ACL is not shown in properties

- Package header and body are not shown in properties

- System package? is not shown in properties

- The header for the RE SQL on a package procedure should read:

-- Package Procedure

(not -- Package Function)

Can you also have someone (Khushboo?) do a code review, if that hasn't
happened already?

Thanks.

On Tue, Aug 16, 2016 at 1:31 PM, Harshal Dhumal
<harshal.dhumal@enterprisedb.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@postgresql.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

Thanks - applied with some minor tweaks to groupings.

On Mon, Aug 22, 2016 at 10:40 AM, Harshal Dhumal
<harshal.dhumal@enterprisedb.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.vashi@enterprisedb.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.vashi@enterprisedb.com> wrote:
>>>
>>>
>>>
>>> On Wed, Aug 17, 2016 at 4:35 PM, Dave Page <dpage@pgadmin.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.dhumal@enterprisedb.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@postgresql.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@postgresql.org)
>>>> To make changes to your subscription:
>>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>
>>>
>>
>
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.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

Hi

On Mon, Aug 22, 2016 at 1:04 PM, Harshal Dhumal
<harshal.dhumal@enterprisedb.com> wrote:
> Hi,
> PFA patch for wrong header and body were generated.
>
> Fixed the header and body sql parsing issue.

Won't this break if I have a package called issues.foo or anything
else that matches the ' is' or ' as'?

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

Thanks, applied.

On Mon, Aug 22, 2016 at 3:23 PM, Harshal Dhumal
<harshal.dhumal@enterprisedb.com> wrote:
> Hi Dave,
>
> --
> Harshal Dhumal
> Software Engineer
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Mon, Aug 22, 2016 at 5:45 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Mon, Aug 22, 2016 at 1:04 PM, Harshal Dhumal
>> <harshal.dhumal@enterprisedb.com> wrote:
>> > Hi,
>> > PFA patch for wrong header and body were generated.
>> >
>> > Fixed the header and body sql parsing issue.
>>
>> Won't this break if I have a package called issues.foo or anything
>> else that matches the ' is' or ' as'?
>
> Yes, this causes issue. I have fixed it in updated patch.
>
> Also fixed one more issue pointed by Khushboo, In some cases if we update
> package header it drops package body. Ideally it should not drop body.
>
>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>
>



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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