Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bw1jD-0001VW-PA for pgadmin-hackers@arkaria.postgresql.org; Mon, 17 Oct 2016 06:48:56 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1bw1jD-0005D2-7r for pgadmin-hackers@arkaria.postgresql.org; Mon, 17 Oct 2016 06:48:55 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1bw1iz-0004xZ-Af for pgadmin-hackers@postgresql.org; Mon, 17 Oct 2016 06:48:41 +0000 Received: from mail-lf0-x233.google.com ([2a00:1450:4010:c07::233]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1bw1iu-0001sy-Vm for pgadmin-hackers@postgresql.org; Mon, 17 Oct 2016 06:48:40 +0000 Received: by mail-lf0-x233.google.com with SMTP id x79so278252834lff.0 for ; Sun, 16 Oct 2016 23:48:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=FIxsk87IlipCLruilEkeKB1+cGMmdK8anDUBo9LvnlM=; b=2Di4wV+l06D7yYBlTlZxs0BEw8eori2EQ2qt8GKwKJS/nfGpajRXjPUkcvlHhA/Z3X h6CUeXFoWP3w+5N55dJw6eYLZ83EQ9hElHLMdZzj3pqk8LDGlvlK+GEopvoUm7BshoXY 2l9P3D+MN1g5MeGc1R3niEstgkUZJKSDxytPDDJMieBeXJrRIOXiCu402jo+VnHP/Zxo t3T0q+XvY27SctEbD7Ek1XhERrqyF+Y7M+ILllYmWyg+ythMyQ9CnpJeU/gevzw2MSLI dcAVGku8ITzle5KVpVGCO/dx1BEY4bKNOAJgfK24jO3VyjPC9VWvTBwmUJsjFxyleEWf b7+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=FIxsk87IlipCLruilEkeKB1+cGMmdK8anDUBo9LvnlM=; b=dUDBE8imJwz5BKsG+SRbGzTMHxmUVz6ix7jA1kOhlH//Oe0vG8Zxb/8XhYB6rtt+P1 AFtImE0ntRya1Vyed5wjJ4S1cMw2Rp1R9QkfZ5sWqKezeoi32+873LVnnbJftBiv7tab KR5WCi8abTLlmCqsZslS/L6G+bCnKlaCvCjSGiSBI2TqAXDkVBSQjHaGWsc1TnIa0Yqo 22dOkNQHjJNx1b5cZaEILk+uzQS2vf9n7dNp7d+HDM8GPwA6nSY9TAFxSeWQACGMsbcU Xaw7ZzYLuWcrLBJ4EPes8zonZDdIzTXbDBTmKdU9u6Pwqc4hpApvK1uTlQAHxML8+Pwz B/hw== X-Gm-Message-State: AA6/9RmEZo7cWyVVNyZ3sLyeo04DBzHHsJ+dwBnummd7tE9+noABcFcwlZkHbOdmwDecYxbNSgwO8fvdqFNeq81H X-Received: by 10.28.60.137 with SMTP id j131mr7478396wma.119.1476686915945; Sun, 16 Oct 2016 23:48:35 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.172.198 with HTTP; Sun, 16 Oct 2016 23:48:05 -0700 (PDT) In-Reply-To: References: From: Surinder Kumar Date: Mon, 17 Oct 2016 12:18:05 +0530 Message-ID: Subject: Re: [pgAdmin4][Patch]: RM1728 - Properties are not refreshing after objects are edited To: Dave Page Cc: pgadmin-hackers Content-Type: multipart/mixed; boundary=001a1148fca211f44f053f09f74e X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --001a1148fca211f44f053f09f74e Content-Type: multipart/alternative; boundary=001a1148fca211f449053f09f74c --001a1148fca211f449053f09f74c Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Sun, Oct 16, 2016 at 7:29 AM, Dave Page 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 multip= le > schemas. > =E2=80=8BI have fixed this issue. Now it will lookup the schema ID against = the type id instead of type name.=E2=80=8B > > 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. > =E2=80=8BIt looks good to me in other nodes. Please find attached patch and review.=E2=80=8B > > Thanks. > > > On Friday, October 14, 2016, Dave Page wrote: > >> Thanks, applied. >> >> On Friday, October 14, 2016, Surinder Kumar < >> surinder.kumar@enterprisedb.com> 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 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 wrote: >>>> > Thanks, applied. >>>> > >>>> > On Fri, Sep 23, 2016 at 12:05 PM, Surinder Kumar >>>> > 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 >>>> wrote: >>>> >>> >>>> >>> Hi >>>> >>> >>>> >>> On Fri, Sep 23, 2016 at 7:39 AM, Surinder Kumar >>>> >>> 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 > > --001a1148fca211f449053f09f74c Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On Sun, Oct 16, 2016 at 7:29 AM= , Dave Page <dpage@pgadmin.org> 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 b= e safe anyway, if there were types with the same name in multiple schemas.<= /div>
= =E2=80=8BI have fixed this issue. Now it will lookup the schema ID against = the type id instead of type name.=E2=80=8B

Actually, it looks like that's an issue when cr= eating a type too - that is also using an unsafe schema lookup.
<= br>
Please fix this ASAP (i.e. Monday) and double check to ensure= we're not doing any more unsafe lookups like this.
<= div class=3D"gmail_default" style=3D"font-size:small">=E2=80=8BIt looks goo= d to me in other nodes.
Please find attached patch and review.=E2=80=8B

Thanks.

On Friday, October 14, 2016, Dave Page <dpage@pgadmin.org> wrote:
Thanks, applied.

On Friday, October 14, 2016, Surind= er Kumar <surinder.kumar@enterprisedb.com> wrote:
= Hi

Following are the issues fixed in nodes:

1) If we create/update a node with non-default schema, It should return se= lected schema id in return response. but default schema id is returned ever= y time due to which it throws error in properties panel.
Fixed in Domains, Collation, Types, Views & Table nod= e.

2) Incorrect parent id of object node is returned from nodes method= due to which wrong parent id is passed while updating object and=C2=A0=
thus node didn't get refreshed.
Fixed in FTS Configuration, FTS Parser no= des.

Also, I have kept changes of first patch which are essential to refr= esh 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 <dpage@pgad= min.org> 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 <dpage@pgadmin.org>= wrote:
> Thanks, applied.
>
> On Fri, Sep 23, 2016 at 12:05 PM, Surinder Kumar
> <surinder.kumar@enterprisedb.com> 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 <dpage@pgadmin.or= g> wrote:
>>>
>>> Hi
>>>
>>> On Fri, Sep 23, 2016 at 7:39 AM, Surinder Kumar
>>> <surinder.kumar@enterprisedb.com> wrote:
>>> > Hi
>>> >
>>> > Issue:
>>> > on updating node, we deselect and then again select the n= ode updated to
>>> > refresh the panel. but it needs some delay of few millise= conds 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 synon= ym 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

En= terpriseDB UK: ht= tp://www.enterprisedb.com
The Enterprise PostgreSQL Company


--001a1148fca211f449053f09f74c-- --001a1148fca211f44f053f09f74e Content-Type: application/octet-stream; name="fix_unsafe_schema_lookup_issue_in_type.patch" Content-Disposition: attachment; filename="fix_unsafe_schema_lookup_issue_in_type.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_iudp8h1d0 ZGlmZiAtLWdpdCBhL3dlYi9wZ2FkbWluL2Jyb3dzZXIvc2VydmVyX2dyb3Vw cy9zZXJ2ZXJzL2RhdGFiYXNlcy9zY2hlbWFzL3R5cGVzL19faW5pdF9fLnB5 IGIvd2ViL3BnYWRtaW4vYnJvd3Nlci9zZXJ2ZXJfZ3JvdXBzL3NlcnZlcnMv ZGF0YWJhc2VzL3NjaGVtYXMvdHlwZXMvX19pbml0X18ucHkKaW5kZXggZTZi N2YzYy4uMjRiMDE2YSAxMDA2NDQKLS0tIGEvd2ViL3BnYWRtaW4vYnJvd3Nl ci9zZXJ2ZXJfZ3JvdXBzL3NlcnZlcnMvZGF0YWJhc2VzL3NjaGVtYXMvdHlw ZXMvX19pbml0X18ucHkKKysrIGIvd2ViL3BnYWRtaW4vYnJvd3Nlci9zZXJ2 ZXJfZ3JvdXBzL3NlcnZlcnMvZGF0YWJhc2VzL3NjaGVtYXMvdHlwZXMvX19p bml0X18ucHkKQEAgLTkwNiwxMiArOTA2LDEzIEBAIGNsYXNzIFR5cGVWaWV3 KFBHQ2hpbGROb2RlVmlldywgRGF0YVR5cGVSZWFkZXIpOgogICAgICAgICAg ICAgaWYgbm90IHN0YXR1czoKICAgICAgICAgICAgICAgICByZXR1cm4gaW50 ZXJuYWxfc2VydmVyX2Vycm9yKGVycm9ybXNnPXJlcykKIAotICAgICAgICAg ICAgIyB3ZSBuZWVkIHNjaWQgdG8gdXBkYXRlIGluIGJyb3dzZXIgdHJlZQot ICAgICAgICAgICAgU1FMID0gcmVuZGVyX3RlbXBsYXRlKCIvIi5qb2luKFtz ZWxmLnRlbXBsYXRlX3BhdGgsCi0gICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgJ2dldF9zY2lkLnNxbCddKSwgdG5hbWU9ZGF0YVsnbmFtZSdd KQotICAgICAgICAgICAgc3RhdHVzLCBzY2lkID0gc2VsZi5jb25uLmV4ZWN1 dGVfc2NhbGFyKFNRTCkKLSAgICAgICAgICAgIGlmIG5vdCBzdGF0dXM6Ci0g ICAgICAgICAgICAgICAgcmV0dXJuIGludGVybmFsX3NlcnZlcl9lcnJvcihl cnJvcm1zZz1zY2lkKQorICAgICAgICAgICAgaWYgJ3NjaGVtYScgaW4gZGF0 YToKKyAgICAgICAgICAgICAgICAjIHdlIG5lZWQgc2NpZCB0byB1cGRhdGUg aW4gYnJvd3NlciB0cmVlCisgICAgICAgICAgICAgICAgU1FMID0gcmVuZGVy X3RlbXBsYXRlKCIvIi5qb2luKFtzZWxmLnRlbXBsYXRlX3BhdGgsCisgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICdnZXRfc2NpZC5z cWwnXSksIHNjaGVtYT1kYXRhWydzY2hlbWEnXSkKKyAgICAgICAgICAgICAg ICBzdGF0dXMsIHNjaWQgPSBzZWxmLmNvbm4uZXhlY3V0ZV9zY2FsYXIoU1FM KQorICAgICAgICAgICAgICAgIGlmIG5vdCBzdGF0dXM6CisgICAgICAgICAg ICAgICAgICAgIHJldHVybiBpbnRlcm5hbF9zZXJ2ZXJfZXJyb3IoZXJyb3Jt c2c9c2NpZCkKIAogICAgICAgICAgICAgIyB3ZSBuZWVkIG9pZCB0byB0byBh ZGQgb2JqZWN0IGluIHRyZWUgYXQgYnJvd3NlcgogICAgICAgICAgICAgU1FM ID0gcmVuZGVyX3RlbXBsYXRlKCIvIi5qb2luKFtzZWxmLnRlbXBsYXRlX3Bh dGgsCkBAIC05NTYsNyArOTU3LDcgQEAgY2xhc3MgVHlwZVZpZXcoUEdDaGls ZE5vZGVWaWV3LCBEYXRhVHlwZVJlYWRlcik6CiAgICAgICAgICAgICAgICAg cmV0dXJuIGludGVybmFsX3NlcnZlcl9lcnJvcihlcnJvcm1zZz1yZXMpCiAK ICAgICAgICAgICAgIFNRTCA9IHJlbmRlcl90ZW1wbGF0ZSgiLyIuam9pbihb c2VsZi50ZW1wbGF0ZV9wYXRoLAotICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICdnZXRfc2NpZC5zcWwnXSksIHRuYW1lPWRhdGFbJ25hbWUn XSkKKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAnZ2V0X3Nj aWQuc3FsJ10pLCB0aWQ9dGlkKQogCiAgICAgICAgICAgICAjIEdldCB1cGRh dGVkIHNjaGVtYSBvaWQKICAgICAgICAgICAgIHN0YXR1cywgc2NpZCA9IHNl bGYuY29ubi5leGVjdXRlX3NjYWxhcihTUUwpCmRpZmYgLS1naXQgYS93ZWIv cGdhZG1pbi9icm93c2VyL3NlcnZlcl9ncm91cHMvc2VydmVycy9kYXRhYmFz ZXMvc2NoZW1hcy90eXBlcy90ZW1wbGF0ZXMvdHlwZS9zcWwvOS4xX3BsdXMv Z2V0X3NjaWQuc3FsIGIvd2ViL3BnYWRtaW4vYnJvd3Nlci9zZXJ2ZXJfZ3Jv dXBzL3NlcnZlcnMvZGF0YWJhc2VzL3NjaGVtYXMvdHlwZXMvdGVtcGxhdGVz L3R5cGUvc3FsLzkuMV9wbHVzL2dldF9zY2lkLnNxbAppbmRleCA2OTZhMjA1 Li4wNjdhOTg2IDEwMDY0NAotLS0gYS93ZWIvcGdhZG1pbi9icm93c2VyL3Nl cnZlcl9ncm91cHMvc2VydmVycy9kYXRhYmFzZXMvc2NoZW1hcy90eXBlcy90 ZW1wbGF0ZXMvdHlwZS9zcWwvOS4xX3BsdXMvZ2V0X3NjaWQuc3FsCisrKyBi L3dlYi9wZ2FkbWluL2Jyb3dzZXIvc2VydmVyX2dyb3Vwcy9zZXJ2ZXJzL2Rh dGFiYXNlcy9zY2hlbWFzL3R5cGVzL3RlbXBsYXRlcy90eXBlL3NxbC85LjFf cGx1cy9nZXRfc2NpZC5zcWwKQEAgLTEsNiArMSwxNSBAQAoreyUgaWYgdGlk ICV9CiBTRUxFQ1QKICAgICB0LnR5cG5hbWVzcGFjZSBhcyBzY2lkCiBGUk9N CiAgICAgcGdfdHlwZSB0CiBXSEVSRQotICAgIHQudHlwbmFtZSA9IHt7dG5h bWV8cXRMaXRlcmFsfX06OnRleHQ7CisgICAgdC5vaWQgPSB7e3RpZH19Ojpv aWQ7Cit7JSBlbHNlICV9CitTRUxFQ1QKKyAgICBucy5vaWQgYXMgc2NpZAor RlJPTQorICAgIHBnX25hbWVzcGFjZSBucworV0hFUkUKKyAgICBucy5uc3Bu YW1lID0ge3tzY2hlbWF8cXRMaXRlcmFsfX06OnRleHQ7Cit7JSBlbmRpZiAl fQo= --001a1148fca211f44f053f09f74e Content-Type: text/plain Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 -- Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers --001a1148fca211f44f053f09f74e--