public inbox for [email protected]  
help / color / mirror / Atom feed
From: Surinder Kumar <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch]: RM1728 - Properties are not refreshing after objects are edited
Date: Mon, 17 Oct 2016 12:18:05 +0530
Message-ID: <CAM5-9D9Yjg-oG3j7FyzSqifYcUMhwaFN_gWv7-9gss0Nkor25Q@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxoxYYna9M5SRwgwb3bN4o-f867E5g8fZp93HgNaT-Sg2pw@mail.gmail.com>
References: <CAM5-9D9n8nvcH44nSGKmMLfCDSV-ka5mrmcEKEdaPJN+zy5CHw@mail.gmail.com>
	<CA+OCxoxwp0_fAaOjReySQAOAtXnsHxQ=--K08Scayfpy=gka9Q@mail.gmail.com>
	<CAM5-9D9s4sz_3dC9Hr6L_O6qEyB5qWE-GjNBvvS26rhguEWCEg@mail.gmail.com>
	<CA+OCxowiq3KZz+86J9BWOLwk=F1c-Cx9FOvG5QCw2awHUK-ucA@mail.gmail.com>
	<CA+OCxoxaYdKVbmEN-zJG5N_wrXViznkngdHwOpM563Jt+Vxe0Q@mail.gmail.com>
	<CAM5-9D9Pbmg4MQdsOqzHeeuWWbZvBCLvzeauhQ6U8vtwSrJLow@mail.gmail.com>
	<CA+OCxoz4k0yE6RzyHcSaCfXLVJHndA17yXoOA1Ae38A6yV5LWw@mail.gmail.com>
	<CA+OCxoxYYna9M5SRwgwb3bN4o-f867E5g8fZp93HgNaT-Sg2pw@mail.gmail.com>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

On Sun, Oct 16, 2016 at 7:29 AM, Dave Page <[email protected]> wrote:

> Hi
>
> I just found a case where this patch is broken - if you update the comment
> on a type, it looks like it tried to lookup the schema ID using the type
> name, which a) isn't in the posted data so gives a 500 response, and b)
> wouldn't be safe anyway, if there were types with the same name in multiple
> schemas.
>
​I have fixed this issue. Now it will lookup the schema ID against the type
id instead of type name.​

>
> Actually, it looks like that's an issue when creating a type too - that is
> also using an unsafe schema lookup.
>
> Please fix this ASAP (i.e. Monday) and double check to ensure we're not
> doing any more unsafe lookups like this.
>
​It looks good to me in other nodes.
Please find attached patch and review.​

>
> Thanks.
>
>
> On Friday, October 14, 2016, Dave Page <[email protected]> wrote:
>
>> Thanks, applied.
>>
>> On Friday, October 14, 2016, Surinder Kumar <
>> [email protected]> wrote:
>>
>>> Hi
>>>
>>> *Following are the issues fixed in nodes:*
>>>
>>> 1) If we create/update a node with non-default schema, It should return
>>> selected schema id in return response. but default schema id is returned
>>> every time due to which it throws error in properties panel.
>>> Fixed in Domains, Collation, Types, Views & Table node.
>>>
>>> 2) Incorrect parent id of object node is returned from *nodes method*
>>> due to which wrong parent id is passed while updating object and
>>> thus node didn't get refreshed.
>>> Fixed in FTS Configuration, FTS Parser nodes.
>>>
>>> Also, I have kept changes of first patch which are essential to refresh
>>> node every time. Without that patch nodes properties panel updates only
>>> sometimes.
>>>
>>> Please find attached patch. Please review and let me know for comments.
>>>
>>> Thanks
>>> Surinder Kumar
>>>
>>>
>>>
>>> On Fri, Sep 23, 2016 at 6:00 PM, Dave Page <[email protected]> wrote:
>>>
>>>> Umm, no it wasn't - sorry.
>>>>
>>>> I see the same issue with Types. Can you fix that, and check all other
>>>> nodes as well please?
>>>>
>>>> Thanks.
>>>>
>>>> On Fri, Sep 23, 2016 at 1:29 PM, Dave Page <[email protected]> wrote:
>>>> > Thanks, applied.
>>>> >
>>>> > On Fri, Sep 23, 2016 at 12:05 PM, Surinder Kumar
>>>> > <[email protected]> wrote:
>>>> >> Hi,
>>>> >>
>>>> >> Please find updated patch with changes:
>>>> >> 1) On debugging through JS files, the issue was in synonym update
>>>> method
>>>> >> which wasn't returning node object.
>>>> >> 2) retrieving schema name in node.sql for creating node object in
>>>> update
>>>> >> method.
>>>> >>
>>>> >> Please review and let me know for comments.
>>>> >>
>>>> >> On Fri, Sep 23, 2016 at 2:44 PM, Dave Page <[email protected]>
>>>> wrote:
>>>> >>>
>>>> >>> Hi
>>>> >>>
>>>> >>> On Fri, Sep 23, 2016 at 7:39 AM, Surinder Kumar
>>>> >>> <[email protected]> wrote:
>>>> >>> > Hi
>>>> >>> >
>>>> >>> > Issue:
>>>> >>> > on updating node, we deselect and then again select the node
>>>> updated to
>>>> >>> > refresh the panel. but it needs some delay of few milliseconds
>>>> between
>>>> >>> > deselect and select to fix this issue.
>>>> >>> >
>>>> >>> > Please find attached patch and review.
>>>> >>>
>>>> >>> This does not resolve the issue for me. I tested using a synonym to
>>>> a
>>>> >>> package on EPAS 9.5, by changing the target package name.
>>>> >>>
>>>> >>>
>>>> >>> --
>>>> >>> 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
>>>>
>>>>
>>>>
>>>> --
>>>> 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
>>
>>
>
> --
> 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers


Attachments:

  [application/octet-stream] fix_unsafe_schema_lookup_issue_in_type.patch (2.6K, 3-fix_unsafe_schema_lookup_issue_in_type.patch)
  download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py
index e6b7f3c..24b016a 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py
@@ -906,12 +906,13 @@ class TypeView(PGChildNodeView, DataTypeReader):
             if not status:
                 return internal_server_error(errormsg=res)
 
-            # we need scid to update in browser tree
-            SQL = render_template("/".join([self.template_path,
-                                  'get_scid.sql']), tname=data['name'])
-            status, scid = self.conn.execute_scalar(SQL)
-            if not status:
-                return internal_server_error(errormsg=scid)
+            if 'schema' in data:
+                # we need scid to update in browser tree
+                SQL = render_template("/".join([self.template_path,
+                                      'get_scid.sql']), schema=data['schema'])
+                status, scid = self.conn.execute_scalar(SQL)
+                if not status:
+                    return internal_server_error(errormsg=scid)
 
             # we need oid to to add object in tree at browser
             SQL = render_template("/".join([self.template_path,
@@ -956,7 +957,7 @@ class TypeView(PGChildNodeView, DataTypeReader):
                 return internal_server_error(errormsg=res)
 
             SQL = render_template("/".join([self.template_path,
-                                  'get_scid.sql']), tname=data['name'])
+                                  'get_scid.sql']), tid=tid)
 
             # Get updated schema oid
             status, scid = self.conn.execute_scalar(SQL)
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/get_scid.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/get_scid.sql
index 696a205..067a986 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/get_scid.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/templates/type/sql/9.1_plus/get_scid.sql
@@ -1,6 +1,15 @@
+{% if tid %}
 SELECT
     t.typnamespace as scid
 FROM
     pg_type t
 WHERE
-    t.typname = {{tname|qtLiteral}}::text;
+    t.oid = {{tid}}::oid;
+{% else %}
+SELECT
+    ns.oid as scid
+FROM
+    pg_namespace ns
+WHERE
+    ns.nspname = {{schema|qtLiteral}}::text;
+{% endif %}


view thread (12+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: [pgAdmin4][Patch]: RM1728 - Properties are not refreshing after objects are edited
  In-Reply-To: <CAM5-9D9Yjg-oG3j7FyzSqifYcUMhwaFN_gWv7-9gss0Nkor25Q@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox