Thread: PATCH: Schema/Catalog Node [pgAdmin4]

PATCH: Schema/Catalog Node [pgAdmin4]

From
Murtuza Zabuawala
Date:
Hi,

PFA patch for schema/catalog node.


--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment

Re: PATCH: Schema/Catalog Node [pgAdmin4]

From
Neel Patel
Date:
Hi Murtuza,

Please find below review comments.

- Remove the whitespace from the patch file as we are getting below warnings while applying the patch file.

   schema_catalog_node_v1.patch:1500: trailing whitespace.
   FROM pg_namespace nsp 
   schema_catalog_node_v1.patch:1501: trailing whitespace.
   LEFT OUTER JOIN 
   schema_catalog_node_v1.patch:1672: trailing whitespace.
   FROM pg_namespace nsp 
   schema_catalog_node_v1.patch:1673: trailing whitespace.
   LEFT OUTER JOIN 
   schema_catalog_node_v1.patch:2353: trailing whitespace.
(SELECT defaclobjtype, aclexplode(defaclacl) as acl 
   warning: squelched 7 whitespace errors
   warning: 12 lines add whitespace errors.

- Some of the Properties are not getting displayed under Properties panel of schema. Make sure to display all the properties displayed in pgadmin3.
  e.g. System Schema 
- We are getting error at python side saying "ValueError: View function did not return a response" while editing the schema node.
- Default Privileges are not getting displayed under SQL panel.
- When we connect to PostgreSQL 9.1 database then we are getting error fail to execute query as below. Below error will be for both schema and 
  catalog node.

    ERROR pgadmin: Failed to execute query (execute_2darray) for the server #3 - DB:postgres (Query-id: 1019298):
    Error Message:relation "pg_shseclabel" does not exist
    LINE 11: ... (SELECT array_agg(provider || '=' || label) FROM pg_shsecla...

Thanks,
Neel Patel

On Mon, Feb 8, 2016 at 2:25 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA patch for schema/catalog node.


--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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


Re: PATCH: Schema/Catalog Node [pgAdmin4]

From
Murtuza Zabuawala
Date:
Hi,

Thanks Neel for reviewing.

PFA updated patch which will fix mentioned issues.


Regards,
Murtuza

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Feb 11, 2016 at 11:58 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:
Hi Murtuza,

Please find below review comments.

- Remove the whitespace from the patch file as we are getting below warnings while applying the patch file.

   schema_catalog_node_v1.patch:1500: trailing whitespace.
   FROM pg_namespace nsp 
   schema_catalog_node_v1.patch:1501: trailing whitespace.
   LEFT OUTER JOIN 
   schema_catalog_node_v1.patch:1672: trailing whitespace.
   FROM pg_namespace nsp 
   schema_catalog_node_v1.patch:1673: trailing whitespace.
   LEFT OUTER JOIN 
   schema_catalog_node_v1.patch:2353: trailing whitespace.
(SELECT defaclobjtype, aclexplode(defaclacl) as acl 
   warning: squelched 7 whitespace errors
   warning: 12 lines add whitespace errors.

- Some of the Properties are not getting displayed under Properties panel of schema. Make sure to display all the properties displayed in pgadmin3.
  e.g. System Schema 
- We are getting error at python side saying "ValueError: View function did not return a response" while editing the schema node.
- Default Privileges are not getting displayed under SQL panel.
- When we connect to PostgreSQL 9.1 database then we are getting error fail to execute query as below. Below error will be for both schema and 
  catalog node.

    ERROR pgadmin: Failed to execute query (execute_2darray) for the server #3 - DB:postgres (Query-id: 1019298):
    Error Message:relation "pg_shseclabel" does not exist
    LINE 11: ... (SELECT array_agg(provider || '=' || label) FROM pg_shsecla...

Thanks,
Neel Patel

On Mon, Feb 8, 2016 at 2:25 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA patch for schema/catalog node.


--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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



Attachment

Re: PATCH: Schema/Catalog Node [pgAdmin4]

From
Murtuza Zabuawala
Date:
Hi All,

PFA updated patch for schema/Catalog node which includes dependent/dependancy route handling code.

Reagrds,
Murtuza

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Feb 11, 2016 at 4:16 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Neel for reviewing.

PFA updated patch which will fix mentioned issues.


Regards,
Murtuza

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Feb 11, 2016 at 11:58 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:
Hi Murtuza,

Please find below review comments.

- Remove the whitespace from the patch file as we are getting below warnings while applying the patch file.

   schema_catalog_node_v1.patch:1500: trailing whitespace.
   FROM pg_namespace nsp 
   schema_catalog_node_v1.patch:1501: trailing whitespace.
   LEFT OUTER JOIN 
   schema_catalog_node_v1.patch:1672: trailing whitespace.
   FROM pg_namespace nsp 
   schema_catalog_node_v1.patch:1673: trailing whitespace.
   LEFT OUTER JOIN 
   schema_catalog_node_v1.patch:2353: trailing whitespace.
(SELECT defaclobjtype, aclexplode(defaclacl) as acl 
   warning: squelched 7 whitespace errors
   warning: 12 lines add whitespace errors.

- Some of the Properties are not getting displayed under Properties panel of schema. Make sure to display all the properties displayed in pgadmin3.
  e.g. System Schema 
- We are getting error at python side saying "ValueError: View function did not return a response" while editing the schema node.
- Default Privileges are not getting displayed under SQL panel.
- When we connect to PostgreSQL 9.1 database then we are getting error fail to execute query as below. Below error will be for both schema and 
  catalog node.

    ERROR pgadmin: Failed to execute query (execute_2darray) for the server #3 - DB:postgres (Query-id: 1019298):
    Error Message:relation "pg_shseclabel" does not exist
    LINE 11: ... (SELECT array_agg(provider || '=' || label) FROM pg_shsecla...

Thanks,
Neel Patel

On Mon, Feb 8, 2016 at 2:25 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA patch for schema/catalog node.


--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




Attachment

Re: PATCH: Schema/Catalog Node [pgAdmin4]

From
Murtuza Zabuawala
Date:
Hi All,

PFA updated patch for Schema/Catalog node with minor enhancements in code.

Regards,
Murtuza

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Tue, Feb 23, 2016 at 1:32 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi All,

PFA updated patch for schema/Catalog node which includes dependent/dependancy route handling code.

Reagrds,
Murtuza

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Feb 11, 2016 at 4:16 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Neel for reviewing.

PFA updated patch which will fix mentioned issues.


Regards,
Murtuza

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Feb 11, 2016 at 11:58 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:
Hi Murtuza,

Please find below review comments.

- Remove the whitespace from the patch file as we are getting below warnings while applying the patch file.

   schema_catalog_node_v1.patch:1500: trailing whitespace.
   FROM pg_namespace nsp 
   schema_catalog_node_v1.patch:1501: trailing whitespace.
   LEFT OUTER JOIN 
   schema_catalog_node_v1.patch:1672: trailing whitespace.
   FROM pg_namespace nsp 
   schema_catalog_node_v1.patch:1673: trailing whitespace.
   LEFT OUTER JOIN 
   schema_catalog_node_v1.patch:2353: trailing whitespace.
(SELECT defaclobjtype, aclexplode(defaclacl) as acl 
   warning: squelched 7 whitespace errors
   warning: 12 lines add whitespace errors.

- Some of the Properties are not getting displayed under Properties panel of schema. Make sure to display all the properties displayed in pgadmin3.
  e.g. System Schema 
- We are getting error at python side saying "ValueError: View function did not return a response" while editing the schema node.
- Default Privileges are not getting displayed under SQL panel.
- When we connect to PostgreSQL 9.1 database then we are getting error fail to execute query as below. Below error will be for both schema and 
  catalog node.

    ERROR pgadmin: Failed to execute query (execute_2darray) for the server #3 - DB:postgres (Query-id: 1019298):
    Error Message:relation "pg_shseclabel" does not exist
    LINE 11: ... (SELECT array_agg(provider || '=' || label) FROM pg_shsecla...

Thanks,
Neel Patel

On Mon, Feb 8, 2016 at 2:25 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA patch for schema/catalog node.


--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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





Attachment

Re: PATCH: Schema/Catalog Node [pgAdmin4]

From
Ashesh Vashi
Date:
Hi Dave,

As discussed, I have worked on this patch to improve some code level changes.
(because - Murtuza was not available due to some other engagement.)

Can you please review it, and share your feedback?

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi




On Tue, Feb 23, 2016 at 5:41 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi All,

PFA updated patch for Schema/Catalog node with minor enhancements in code.

Regards,
Murtuza

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Tue, Feb 23, 2016 at 1:32 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi All,

PFA updated patch for schema/Catalog node which includes dependent/dependancy route handling code.

Reagrds,
Murtuza

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Feb 11, 2016 at 4:16 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Neel for reviewing.

PFA updated patch which will fix mentioned issues.


Regards,
Murtuza

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Feb 11, 2016 at 11:58 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:
Hi Murtuza,

Please find below review comments.

- Remove the whitespace from the patch file as we are getting below warnings while applying the patch file.

   schema_catalog_node_v1.patch:1500: trailing whitespace.
   FROM pg_namespace nsp 
   schema_catalog_node_v1.patch:1501: trailing whitespace.
   LEFT OUTER JOIN 
   schema_catalog_node_v1.patch:1672: trailing whitespace.
   FROM pg_namespace nsp 
   schema_catalog_node_v1.patch:1673: trailing whitespace.
   LEFT OUTER JOIN 
   schema_catalog_node_v1.patch:2353: trailing whitespace.
(SELECT defaclobjtype, aclexplode(defaclacl) as acl 
   warning: squelched 7 whitespace errors
   warning: 12 lines add whitespace errors.

- Some of the Properties are not getting displayed under Properties panel of schema. Make sure to display all the properties displayed in pgadmin3.
  e.g. System Schema 
- We are getting error at python side saying "ValueError: View function did not return a response" while editing the schema node.
- Default Privileges are not getting displayed under SQL panel.
- When we connect to PostgreSQL 9.1 database then we are getting error fail to execute query as below. Below error will be for both schema and 
  catalog node.

    ERROR pgadmin: Failed to execute query (execute_2darray) for the server #3 - DB:postgres (Query-id: 1019298):
    Error Message:relation "pg_shseclabel" does not exist
    LINE 11: ... (SELECT array_agg(provider || '=' || label) FROM pg_shsecla...

Thanks,
Neel Patel

On Mon, Feb 8, 2016 at 2:25 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA patch for schema/catalog node.


--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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


Attachment

Re: PATCH: Schema/Catalog Node [pgAdmin4]

From
Dave Page
Date:
Hi

On Sun, Feb 28, 2016 at 6:49 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Hi Dave,

As discussed, I have worked on this patch to improve some code level changes.
(because - Murtuza was not available due to some other engagement.)

Can you please review it, and share your feedback?

I think it's  mostly there. I've attached an updated patch where I've fixed a few minor issues as I read through the code. The following (likely simple) issues need to be fixed:

- Dropping a schema fails with an error message of "schema/pg/9.2_plus/sql/get_name.sql". 

- Creating a schema appears to fail with "'data' is undefined", however the schema is created, it's just that the dialogue doesn't close and the new schema isn't added to the tree.

- There is some discrepancy between default privileges as displayed on the properties summary, the edit dialogue, and the RE-SQL. As you can see in the screenshot, the SQL just GRANTS ALL, and the properties panel doesn't show anything.

Thanks.

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

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

Re: PATCH: Schema/Catalog Node [pgAdmin4]

From
Ashesh Vashi
Date:

Hi Dave,


On Thu, Mar 3, 2016 at 8:27 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Sun, Feb 28, 2016 at 6:49 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Hi Dave,

As discussed, I have worked on this patch to improve some code level changes.
(because - Murtuza was not available due to some other engagement.)

Can you please review it, and share your feedback?

I think it's  mostly there. I've attached an updated patch where I've fixed a few minor issues as I read through the code. The following (likely simple) issues need to be fixed:

- Dropping a schema fails with an error message of "schema/pg/9.2_plus/sql/get_name.sql". 
Done. 

- Creating a schema appears to fail with "'data' is undefined", however the schema is created, it's just that the dialogue doesn't close and the new schema isn't added to the tree.
Done. 

- There is some discrepancy between default privileges as displayed on the properties summary, the edit dialogue, and the RE-SQL. As you can see in the screenshot, the SQL just GRANTS ALL, and the properties panel doesn't show anything.
Yes - there were some typos in the schema/catalog node implementation, which I have resolved now.

Please find the updated patch.

--
Thanks & Regards,
Ashesh Vashi


Thanks.

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

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

Attachment

Re: PATCH: Schema/Catalog Node [pgAdmin4]

From
Dave Page
Date:


On Tue, Mar 8, 2016 at 2:18 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Tue, Mar 8, 2016 at 7:36 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Hi Dave,

On Tue, Mar 8, 2016 at 12:20 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Hi Dave,


On Thu, Mar 3, 2016 at 8:27 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Sun, Feb 28, 2016 at 6:49 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Hi Dave,

As discussed, I have worked on this patch to improve some code level changes.
(because - Murtuza was not available due to some other engagement.)

Can you please review it, and share your feedback?

I think it's  mostly there. I've attached an updated patch where I've fixed a few minor issues as I read through the code. The following (likely simple) issues need to be fixed:

- Dropping a schema fails with an error message of "schema/pg/9.2_plus/sql/get_name.sql". 
Done. 

- Creating a schema appears to fail with "'data' is undefined", however the schema is created, it's just that the dialogue doesn't close and the new schema isn't added to the tree.
Done. 

- There is some discrepancy between default privileges as displayed on the properties summary, the edit dialogue, and the RE-SQL. As you can see in the screenshot, the SQL just GRANTS ALL, and the properties panel doesn't show anything.
Yes - there were some typos in the schema/catalog node implementation, which I have resolved now.

Please find the updated patch.
One more updated patch:
Some of the catalogs will not have all the schema child objects.
Hence - they will need to check certain thing likes they're not being loading in the catalog with such property (i.e. pg_catalog).
As per my conversation with Murtuza, who has already implemented catalog_obejcts for this kind of catalogs, these objects are only supported for catalogs like information_schema (and, PPAS specific dbo, sys).
To ease the work, I have introduced a class name SchemaChildModule, which does that job for us.
Please find the patch as per his input.


Can you split out the new changes please? I just spent 30 minutes tweaking the last patch.
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

Re: PATCH: Schema/Catalog Node [pgAdmin4]

From
Ashesh Vashi
Date:
Hi Dave,

On Tue, Mar 8, 2016 at 12:20 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Hi Dave,


On Thu, Mar 3, 2016 at 8:27 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Sun, Feb 28, 2016 at 6:49 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Hi Dave,

As discussed, I have worked on this patch to improve some code level changes.
(because - Murtuza was not available due to some other engagement.)

Can you please review it, and share your feedback?

I think it's  mostly there. I've attached an updated patch where I've fixed a few minor issues as I read through the code. The following (likely simple) issues need to be fixed:

- Dropping a schema fails with an error message of "schema/pg/9.2_plus/sql/get_name.sql". 
Done. 

- Creating a schema appears to fail with "'data' is undefined", however the schema is created, it's just that the dialogue doesn't close and the new schema isn't added to the tree.
Done. 

- There is some discrepancy between default privileges as displayed on the properties summary, the edit dialogue, and the RE-SQL. As you can see in the screenshot, the SQL just GRANTS ALL, and the properties panel doesn't show anything.
Yes - there were some typos in the schema/catalog node implementation, which I have resolved now.

Please find the updated patch.
One more updated patch:
Some of the catalogs will not have all the schema child objects.
Hence - they will need to check certain thing likes they're not being loading in the catalog with such property (i.e. pg_catalog).
To ease the work, I have introduced a class name SchemaChildModule, which does that job for us.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company



--
Thanks & Regards,
Ashesh Vashi


Thanks.

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

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


Attachment

Re: PATCH: Schema/Catalog Node [pgAdmin4]

From
Ashesh Vashi
Date:
On Tue, Mar 8, 2016 at 7:36 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Hi Dave,

On Tue, Mar 8, 2016 at 12:20 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Hi Dave,


On Thu, Mar 3, 2016 at 8:27 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Sun, Feb 28, 2016 at 6:49 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Hi Dave,

As discussed, I have worked on this patch to improve some code level changes.
(because - Murtuza was not available due to some other engagement.)

Can you please review it, and share your feedback?

I think it's  mostly there. I've attached an updated patch where I've fixed a few minor issues as I read through the code. The following (likely simple) issues need to be fixed:

- Dropping a schema fails with an error message of "schema/pg/9.2_plus/sql/get_name.sql". 
Done. 

- Creating a schema appears to fail with "'data' is undefined", however the schema is created, it's just that the dialogue doesn't close and the new schema isn't added to the tree.
Done. 

- There is some discrepancy between default privileges as displayed on the properties summary, the edit dialogue, and the RE-SQL. As you can see in the screenshot, the SQL just GRANTS ALL, and the properties panel doesn't show anything.
Yes - there were some typos in the schema/catalog node implementation, which I have resolved now.

Please find the updated patch.
One more updated patch:
Some of the catalogs will not have all the schema child objects.
Hence - they will need to check certain thing likes they're not being loading in the catalog with such property (i.e. pg_catalog).
As per my conversation with Murtuza, who has already implemented catalog_obejcts for this kind of catalogs, these objects are only supported for catalogs like information_schema (and, PPAS specific dbo, sys).
To ease the work, I have introduced a class name SchemaChildModule, which does that job for us.
Please find the patch as per his input.

 

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company



--
Thanks & Regards,
Ashesh Vashi


Thanks.

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

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



Attachment

Re: PATCH: Schema/Catalog Node [pgAdmin4]

From
Ashesh Vashi
Date:
On Tue, Mar 8, 2016 at 8:08 PM, Dave Page <dpage@pgadmin.org> wrote:


On Tue, Mar 8, 2016 at 2:18 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Tue, Mar 8, 2016 at 7:36 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Hi Dave,

On Tue, Mar 8, 2016 at 12:20 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Hi Dave,


On Thu, Mar 3, 2016 at 8:27 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Sun, Feb 28, 2016 at 6:49 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Hi Dave,

As discussed, I have worked on this patch to improve some code level changes.
(because - Murtuza was not available due to some other engagement.)

Can you please review it, and share your feedback?

I think it's  mostly there. I've attached an updated patch where I've fixed a few minor issues as I read through the code. The following (likely simple) issues need to be fixed:

- Dropping a schema fails with an error message of "schema/pg/9.2_plus/sql/get_name.sql". 
Done. 

- Creating a schema appears to fail with "'data' is undefined", however the schema is created, it's just that the dialogue doesn't close and the new schema isn't added to the tree.
Done. 

- There is some discrepancy between default privileges as displayed on the properties summary, the edit dialogue, and the RE-SQL. As you can see in the screenshot, the SQL just GRANTS ALL, and the properties panel doesn't show anything.
Yes - there were some typos in the schema/catalog node implementation, which I have resolved now.

Please find the updated patch.
One more updated patch:
Some of the catalogs will not have all the schema child objects.
Hence - they will need to check certain thing likes they're not being loading in the catalog with such property (i.e. pg_catalog).
As per my conversation with Murtuza, who has already implemented catalog_obejcts for this kind of catalogs, these objects are only supported for catalogs like information_schema (and, PPAS specific dbo, sys).
To ease the work, I have introduced a class name SchemaChildModule, which does that job for us.
Please find the patch as per his input.


Can you split out the new changes please? I just spent 30 minutes tweaking the last patch.
Sure.
Here is the patch based on the v10 patch.

--
Thanks & Regards,
Ashesh Vashi 
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

Attachment

Re: PATCH: Schema/Catalog Node [pgAdmin4]

From
Dave Page
Date:
On Tue, Mar 8, 2016 at 2:46 PM, Ashesh Vashi
<ashesh.vashi@enterprisedb.com> wrote:
> On Tue, Mar 8, 2016 at 8:08 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>>
>>
>> On Tue, Mar 8, 2016 at 2:18 PM, Ashesh Vashi
>> <ashesh.vashi@enterprisedb.com> wrote:
>>>
>>> On Tue, Mar 8, 2016 at 7:36 PM, Ashesh Vashi
>>> <ashesh.vashi@enterprisedb.com> wrote:
>>>>
>>>> Hi Dave,
>>>>
>>>> On Tue, Mar 8, 2016 at 12:20 AM, Ashesh Vashi
>>>> <ashesh.vashi@enterprisedb.com> wrote:
>>>>>
>>>>> Hi Dave,
>>>>>
>>>>>
>>>>> On Thu, Mar 3, 2016 at 8:27 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>>>>
>>>>>> Hi
>>>>>>
>>>>>> On Sun, Feb 28, 2016 at 6:49 AM, Ashesh Vashi
>>>>>> <ashesh.vashi@enterprisedb.com> wrote:
>>>>>>>
>>>>>>> Hi Dave,
>>>>>>>
>>>>>>> As discussed, I have worked on this patch to improve some code level
>>>>>>> changes.
>>>>>>> (because - Murtuza was not available due to some other engagement.)
>>>>>>>
>>>>>>> Can you please review it, and share your feedback?
>>>>>>
>>>>>>
>>>>>> I think it's  mostly there. I've attached an updated patch where I've
>>>>>> fixed a few minor issues as I read through the code. The following (likely
>>>>>> simple) issues need to be fixed:
>>>>>>
>>>>>> - Dropping a schema fails with an error message of
>>>>>> "schema/pg/9.2_plus/sql/get_name.sql".
>>>>>
>>>>> Done.
>>>>>>
>>>>>>
>>>>>> - Creating a schema appears to fail with "'data' is undefined",
>>>>>> however the schema is created, it's just that the dialogue doesn't close and
>>>>>> the new schema isn't added to the tree.
>>>>>
>>>>> Done.
>>>>>>
>>>>>>
>>>>>> - There is some discrepancy between default privileges as displayed on
>>>>>> the properties summary, the edit dialogue, and the RE-SQL. As you can see in
>>>>>> the screenshot, the SQL just GRANTS ALL, and the properties panel doesn't
>>>>>> show anything.
>>>>>
>>>>> Yes - there were some typos in the schema/catalog node implementation,
>>>>> which I have resolved now.
>>>>>
>>>>> Please find the updated patch.
>>>>
>>>> One more updated patch:
>>>> Some of the catalogs will not have all the schema child objects.
>>>> Hence - they will need to check certain thing likes they're not being
>>>> loading in the catalog with such property (i.e. pg_catalog).
>>>
>>> As per my conversation with Murtuza, who has already implemented
>>> catalog_obejcts for this kind of catalogs, these objects are only supported
>>> for catalogs like information_schema (and, PPAS specific dbo, sys).
>>>>
>>>> To ease the work, I have introduced a class name SchemaChildModule,
>>>> which does that job for us.
>>>
>>> Please find the patch as per his input.
>>>
>>
>> Can you split out the new changes please? I just spent 30 minutes tweaking
>> the last patch.
>
> Sure.
> Here is the patch based on the v10 patch.

Thanks - patch committed. I made the following changes:

- Removed explicit support for 9.0 and below.
- Hid the default ACLs from the properties list for catalogs.
- Tidied up some of the SQL formatting

Open questions:

- We don't allow default ACLs to be specified when creating a schema
(neither does pgAdmin). Why not? Shouldn't we?

- We create new objects in 2 SQL statements, one that runs create.sql
and one that runs alter.sql to apply ACL, label options and more. I
strongly believe we need to push this into a single statement for all
object types, to ensure creation is completely atomic. Right now, you
can easily get an error by trying to set an unregistered security
label, which keeps the create dialogue open, however the object has
been successfully created.

Thoughts?


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

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


Re: PATCH: Schema/Catalog Node [pgAdmin4]

From
Ashesh Vashi
Date:
On Tue, Mar 8, 2016 at 9:23 PM, Dave Page <dpage@pgadmin.org> wrote:
On Tue, Mar 8, 2016 at 2:46 PM, Ashesh Vashi
<ashesh.vashi@enterprisedb.com> wrote:
> On Tue, Mar 8, 2016 at 8:08 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>>
>>
>> On Tue, Mar 8, 2016 at 2:18 PM, Ashesh Vashi
>> <ashesh.vashi@enterprisedb.com> wrote:
>>>
>>> On Tue, Mar 8, 2016 at 7:36 PM, Ashesh Vashi
>>> <ashesh.vashi@enterprisedb.com> wrote:
>>>>
>>>> Hi Dave,
>>>>
>>>> On Tue, Mar 8, 2016 at 12:20 AM, Ashesh Vashi
>>>> <ashesh.vashi@enterprisedb.com> wrote:
>>>>>
>>>>> Hi Dave,
>>>>>
>>>>>
>>>>> On Thu, Mar 3, 2016 at 8:27 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>>>>
>>>>>> Hi
>>>>>>
>>>>>> On Sun, Feb 28, 2016 at 6:49 AM, Ashesh Vashi
>>>>>> <ashesh.vashi@enterprisedb.com> wrote:
>>>>>>>
>>>>>>> Hi Dave,
>>>>>>>
>>>>>>> As discussed, I have worked on this patch to improve some code level
>>>>>>> changes.
>>>>>>> (because - Murtuza was not available due to some other engagement.)
>>>>>>>
>>>>>>> Can you please review it, and share your feedback?
>>>>>>
>>>>>>
>>>>>> I think it's  mostly there. I've attached an updated patch where I've
>>>>>> fixed a few minor issues as I read through the code. The following (likely
>>>>>> simple) issues need to be fixed:
>>>>>>
>>>>>> - Dropping a schema fails with an error message of
>>>>>> "schema/pg/9.2_plus/sql/get_name.sql".
>>>>>
>>>>> Done.
>>>>>>
>>>>>>
>>>>>> - Creating a schema appears to fail with "'data' is undefined",
>>>>>> however the schema is created, it's just that the dialogue doesn't close and
>>>>>> the new schema isn't added to the tree.
>>>>>
>>>>> Done.
>>>>>>
>>>>>>
>>>>>> - There is some discrepancy between default privileges as displayed on
>>>>>> the properties summary, the edit dialogue, and the RE-SQL. As you can see in
>>>>>> the screenshot, the SQL just GRANTS ALL, and the properties panel doesn't
>>>>>> show anything.
>>>>>
>>>>> Yes - there were some typos in the schema/catalog node implementation,
>>>>> which I have resolved now.
>>>>>
>>>>> Please find the updated patch.
>>>>
>>>> One more updated patch:
>>>> Some of the catalogs will not have all the schema child objects.
>>>> Hence - they will need to check certain thing likes they're not being
>>>> loading in the catalog with such property (i.e. pg_catalog).
>>>
>>> As per my conversation with Murtuza, who has already implemented
>>> catalog_obejcts for this kind of catalogs, these objects are only supported
>>> for catalogs like information_schema (and, PPAS specific dbo, sys).
>>>>
>>>> To ease the work, I have introduced a class name SchemaChildModule,
>>>> which does that job for us.
>>>
>>> Please find the patch as per his input.
>>>
>>
>> Can you split out the new changes please? I just spent 30 minutes tweaking
>> the last patch.
>
> Sure.
> Here is the patch based on the v10 patch.

Thanks - patch committed. I made the following changes:

- Removed explicit support for 9.0 and below.
- Hid the default ACLs from the properties list for catalogs.
- Tidied up some of the SQL formatting

Open questions:

- We don't allow default ACLs to be specified when creating a schema
(neither does pgAdmin). Why not? Shouldn't we?
Hmm.. I don't see any reason, why we should not do it.
We have adopted that from pgAdmin III.

- We create new objects in 2 SQL statements, one that runs create.sql
and one that runs alter.sql to apply ACL, label options and more. I
strongly believe we need to push this into a single statement for all
object types, to ensure creation is completely atomic. Right now, you
can easily get an error by trying to set an unregistered security
label, which keeps the create dialogue open, however the object has
been successfully created.
Agree.
Even if they needed to be created from two, or more separate templates, they should ran together unless there are some statements, which require to run in separate transaction.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi


Thoughts?


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

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

Re: PATCH: Schema/Catalog Node [pgAdmin4]

From
Dave Page
Date:


On Tue, Mar 8, 2016 at 4:34 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Tue, Mar 8, 2016 at 9:23 PM, Dave Page <dpage@pgadmin.org> wrote:
On Tue, Mar 8, 2016 at 2:46 PM, Ashesh Vashi
<ashesh.vashi@enterprisedb.com> wrote:
> On Tue, Mar 8, 2016 at 8:08 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>>
>>
>> On Tue, Mar 8, 2016 at 2:18 PM, Ashesh Vashi
>> <ashesh.vashi@enterprisedb.com> wrote:
>>>
>>> On Tue, Mar 8, 2016 at 7:36 PM, Ashesh Vashi
>>> <ashesh.vashi@enterprisedb.com> wrote:
>>>>
>>>> Hi Dave,
>>>>
>>>> On Tue, Mar 8, 2016 at 12:20 AM, Ashesh Vashi
>>>> <ashesh.vashi@enterprisedb.com> wrote:
>>>>>
>>>>> Hi Dave,
>>>>>
>>>>>
>>>>> On Thu, Mar 3, 2016 at 8:27 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>>>>
>>>>>> Hi
>>>>>>
>>>>>> On Sun, Feb 28, 2016 at 6:49 AM, Ashesh Vashi
>>>>>> <ashesh.vashi@enterprisedb.com> wrote:
>>>>>>>
>>>>>>> Hi Dave,
>>>>>>>
>>>>>>> As discussed, I have worked on this patch to improve some code level
>>>>>>> changes.
>>>>>>> (because - Murtuza was not available due to some other engagement.)
>>>>>>>
>>>>>>> Can you please review it, and share your feedback?
>>>>>>
>>>>>>
>>>>>> I think it's  mostly there. I've attached an updated patch where I've
>>>>>> fixed a few minor issues as I read through the code. The following (likely
>>>>>> simple) issues need to be fixed:
>>>>>>
>>>>>> - Dropping a schema fails with an error message of
>>>>>> "schema/pg/9.2_plus/sql/get_name.sql".
>>>>>
>>>>> Done.
>>>>>>
>>>>>>
>>>>>> - Creating a schema appears to fail with "'data' is undefined",
>>>>>> however the schema is created, it's just that the dialogue doesn't close and
>>>>>> the new schema isn't added to the tree.
>>>>>
>>>>> Done.
>>>>>>
>>>>>>
>>>>>> - There is some discrepancy between default privileges as displayed on
>>>>>> the properties summary, the edit dialogue, and the RE-SQL. As you can see in
>>>>>> the screenshot, the SQL just GRANTS ALL, and the properties panel doesn't
>>>>>> show anything.
>>>>>
>>>>> Yes - there were some typos in the schema/catalog node implementation,
>>>>> which I have resolved now.
>>>>>
>>>>> Please find the updated patch.
>>>>
>>>> One more updated patch:
>>>> Some of the catalogs will not have all the schema child objects.
>>>> Hence - they will need to check certain thing likes they're not being
>>>> loading in the catalog with such property (i.e. pg_catalog).
>>>
>>> As per my conversation with Murtuza, who has already implemented
>>> catalog_obejcts for this kind of catalogs, these objects are only supported
>>> for catalogs like information_schema (and, PPAS specific dbo, sys).
>>>>
>>>> To ease the work, I have introduced a class name SchemaChildModule,
>>>> which does that job for us.
>>>
>>> Please find the patch as per his input.
>>>
>>
>> Can you split out the new changes please? I just spent 30 minutes tweaking
>> the last patch.
>
> Sure.
> Here is the patch based on the v10 patch.

Thanks - patch committed. I made the following changes:

- Removed explicit support for 9.0 and below.
- Hid the default ACLs from the properties list for catalogs.
- Tidied up some of the SQL formatting

Open questions:

- We don't allow default ACLs to be specified when creating a schema
(neither does pgAdmin). Why not? Shouldn't we?
Hmm.. I don't see any reason, why we should not do it.
We have adopted that from pgAdmin III.

- We create new objects in 2 SQL statements, one that runs create.sql
and one that runs alter.sql to apply ACL, label options and more. I
strongly believe we need to push this into a single statement for all
object types, to ensure creation is completely atomic. Right now, you
can easily get an error by trying to set an unregistered security
label, which keeps the create dialogue open, however the object has
been successfully created.
Agree.
Even if they needed to be created from two, or more separate templates, they should ran together unless there are some statements, which require to run in separate transaction.

OK, I added tasks to do both to our internal tracker. 

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

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