Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1f13k7-0003Ro-DR for pgadmin-hackers@arkaria.postgresql.org; Wed, 28 Mar 2018 05:35:27 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1f13k5-0003JV-TB for pgadmin-hackers@arkaria.postgresql.org; Wed, 28 Mar 2018 05:35:25 +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.89) (envelope-from ) id 1f13k5-0003JL-HW for pgadmin-hackers@lists.postgresql.org; Wed, 28 Mar 2018 05:35:25 +0000 Received: from mail-qt0-x230.google.com ([2607:f8b0:400d:c0d::230]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1f13k0-0002VH-JY for pgadmin-hackers@postgresql.org; Wed, 28 Mar 2018 05:35:24 +0000 Received: by mail-qt0-x230.google.com with SMTP id f8so1318224qtg.12 for ; Tue, 27 Mar 2018 22:35:19 -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=mcJRV58sSb5cljacTrdgbHpZZ4nTnIbQ8KXZNtTYkPY=; b=DsAGlwrvJ5yQkNXTXBxOUgVQnRcEdH9h4ZK37sBrOwKW7f5avLxZlJsu384wl1/HWA EOke/tsxdYTHJRTko4IMHZlZwp7bhZNIf7kh+mimhfI4KYwH2RNTWebUwBeLzGlvISoq hCvJKxJ2I4wvGsFukJLLy4hJsGWOTnSATv7i7E0ceq/+HAR+rRrhc3SW90DtqG56YaFW Gbj8amya/u9j6ZVko8UuTWbbkHks3l54D/G/t0mW2WEk1WJ17D9spqXH5UydcGy3FAy7 i0BxZf5f1INtcFOoHLd/c6tDP9TOP/v7O8MYbn86tsiqN/HeSOPSudGlRcSFaF1TRZVP bjgQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=mcJRV58sSb5cljacTrdgbHpZZ4nTnIbQ8KXZNtTYkPY=; b=maArpA7KeEEiO9bksNJ+caXuJclq9jbXHSAYYOtd6kTSi7uaRUlUxncNyYJpai6TVG lcBiuRt2Y8AHNijazDefy4sGWvrRkwYfcUHVbx4x7XR7kmYkkh4YrmFjJR7mFsl4bh67 /ceF+DMNecqDjxkhDA1plZXp3fV5oAXC45e8nFVRFzm5R8T5TwU2UMLHpN5huP5wZ/jG BX1LbumVdbf32Ej1hlt2MFJkKioyuZCzCzBJLioGvHeTi62DWQ0SyUoRBMUD2Utwjtye 1w4aC23NK++g392X5iaBTbyYF/k62PdHYdyNX8/ejYWU8erFZv+D7nlcftMck+zsL+zk LAYQ== X-Gm-Message-State: AElRT7HmDFRL8l/TI8AGVVt6/m5+kTu0GLh8z+0L+s+722PHmZePh9qX gwyT7vE1+h3h0D/BghBJ9uw9sgDDEZF2a2LZb89U2w== X-Google-Smtp-Source: AIpwx4+dJyCyHzCMb/bwdIKfypZB2o09pncT9VJoMIiVGtSaTXho/XL23eLnaySzXUL0KkJiJxjc4jZimrqjMEgycYI= X-Received: by 10.237.47.165 with SMTP id m34mr3407631qtd.178.1522215317898; Tue, 27 Mar 2018 22:35:17 -0700 (PDT) MIME-Version: 1.0 Received: by 10.12.182.1 with HTTP; Tue, 27 Mar 2018 22:35:17 -0700 (PDT) In-Reply-To: References: From: Akshay Joshi Date: Wed, 28 Mar 2018 11:05:17 +0530 Message-ID: Subject: Re: [pgAdmin4][patch]: RM #3090 pgadmin shows misleading "Query returned successfully" with incorrect SQL To: Joao De Almeida Pereira Cc: Dave Page , pgadmin-hackers Content-Type: multipart/alternative; boundary="94eb2c1252244ba4c90568725fd8" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --94eb2c1252244ba4c90568725fd8 Content-Type: text/plain; charset="UTF-8" Hi Joao On Tue, Mar 27, 2018 at 6:53 PM, Joao De Almeida Pereira wrote: > Hi Akshay, > > We were not trying to imply that your fix did not solve the problem, we > were trying to understand the root cause of the problem. > As per my understanding root cause of the problem is "exception_obj. diag.severity" variable was not decoded and used directly, when I debug the code that variable contains the encoded string, so I have just use the function "*self.decode_to_utf8()*" and it works. If you can see the code below to this fix will have all the variables decoded to utf8. > > 1. We were not able to reproduce the problem > We followed your directions in RM, removed your fix but we could not > reproduce the problem. So we could not make sure that the application is in > fact working. This maybe have been because we missed something. > 2. The fix does not tackle the big problem > From what we read on the RM the big problem is "when we throw an exception > the front end is displaying the query successful message". Did you also > understood that from the RM? > We believe this is the real problem behind the RM. > Were you able replicate this behavior? If so we need to address the root > of this. > Yes I understood that from RM, but as I mentioned in my previous email that I haven't encounter that problem while trying to reproduce this. I got "*Not connected to the server*..." error not the "Query returned successfully". This bug has been log 2 months ago, I am not sure but it may possible that we have fixed/change some logic because of that I have seen "Not connected to the server......" error instead of "Query returned successfully". > > > As an aside sniping bugs is fine in some situations but as a general rule > of thumb is a bad approach and creates a blob of code that no one can > manage. A situation like this look like a very promising candidate for > extraction/refactoring in our point of view. In which situation do you > normally refactor or extract code? > > Thanks > Joao > > > On Tue, Mar 27, 2018, 1:31 AM Akshay Joshi > wrote: > >> Hi Joao >> >> On Tue, Mar 27, 2018 at 12:01 AM, Joao De Almeida Pereira < >> jdealmeidapereira@pivotal.io> wrote: >> >>> Hello, >>> We tried to reproduce the issue but we were not capable to reproduce it. >>> What it is strange on the fix is that python is complaining about a >>> different line then the one that was fixed. Maybe this is just a Python >>> thing.... >>> >> >> I have mentioned the steps in RM and I have tried it on Ubuntu 16. >> Python is complaining the exact line where I have fixed the logic. >> >> ex_diag_message = u"{0}: {1}".format( >> self.decode_to_utf8(exception_obj.diag.severity), # exception_obj.diag.severity is not decoded before my fix. >> self.decode_to_utf8(exception_obj.diag.message_primary) >> ) >> >> >> >>> >>> I assume that the fix works, but I would love to see some tests to >>> ensure it is working. Another issue that looks more problematic is the fact >>> that, as per the Redmine issue, when an exception is thrown it sends back a >>> Successful Query message. If this is the case then this fix doesn't look >>> like it is enough to solve the problem. >>> >> >> Yes it works. With the latest code I didn't see Successful Query >> message, it is showing "Not connected to the server .....", but the stack >> trace is same that was mentioned in the RM. >> >>> >>> Thanks >>> Victoria & Joao >>> >>> On Mon, Mar 26, 2018 at 9:00 AM Dave Page wrote: >>> >>>> Thanks, applied. >>>> >>>> On Mon, Mar 26, 2018 at 11:43 AM, Akshay Joshi < >>>> akshay.joshi@enterprisedb.com> wrote: >>>> >>>>> Hi Hackers, >>>>> >>>>> Please find the attached patch to fix RM #3090 pgadmin shows >>>>> misleading "Query returned successfully" with incorrect SQL. >>>>> >>>>> -- >>>>> *Akshay Joshi* >>>>> >>>>> *Sr. Software Architect * >>>>> >>>>> >>>>> >>>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 >>>>> 976-788-8246 <+91%2097678%2088246>* >>>>> >>>> >>>> >>>> >>>> -- >>>> Dave Page >>>> Blog: http://pgsnake.blogspot.com >>>> Twitter: @pgsnake >>>> >>>> EnterpriseDB UK: http://www.enterprisedb.com >>>> The Enterprise PostgreSQL Company >>>> >>> >> >> >> -- >> *Akshay Joshi* >> >> *Sr. Software Architect * >> >> >> >> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246 >> <+91%2097678%2088246>* >> > -- *Akshay Joshi* *Sr. Software Architect * *Phone: +91 20-3058-9517Mobile: +91 976-788-8246* --94eb2c1252244ba4c90568725fd8 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Joao

On = Tue, Mar 27, 2018 at 6:53 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Akshay,

W= e were not trying to imply that your fix did not solve the problem, we were= trying to understand the root cause of the problem.

=C2=A0 =C2=A0 As per my understanding root cause of t= he problem is=C2=A0 "exception_= obj.diag.severity"=C2=A0variable was not decoded an= d used directly, when I debug the code that variable contains the encoded s= tring, so I have just use the function "self.decod= e_to_utf8()" and it works. If you= can see the code below to this fix will have all the variables decoded to = utf8.=C2=A0 =C2=A0 =C2=A0

1. We were not able to reproduce the = problem
We followed your directions in RM, removed your fix but w= e could not reproduce the problem. So we could not make sure that the appli= cation is in fact working. This maybe have been because we missed something= .
2. The fix does not tackle the big problem
From what = we read on the RM the big problem is "when we throw an exception the f= ront end is displaying the query successful message". Did you also und= erstood that from the RM?=C2=A0=C2=A0
We believe this is the real problem= behind the RM.
Were you able replicate this behavior? If so we n= eed to address the root of this.

=C2=A0 =C2=A0 Yes I understood that from RM, bu= t as I mentioned in my previous email that I haven't encounter that pro= blem while trying to reproduce this. I got "Not co= nnected to the server..." error n= ot the "Query returned successfully". This bug has been log 2 mon= ths ago, I am not sure but it may possible that we have fixed/change some l= ogic because of that I have seen "Not connected to the server......&qu= ot; error instead of "Query returned success= fully".=C2=A0=C2=A0=C2=A0


As an aside snipi= ng bugs is fine in some situations but as a general rule of thumb is a bad = approach and creates a blob of code that no one can manage. A situation lik= e this look like a very promising candidate for extraction/refactoring in o= ur point of view. In which situation do you normally refactor or extract co= de?=C2=A0

Thanks
= Joao

=C2=A0=
On Tue, Mar 27, 2018, 1:31 = AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Joao

On Tue, Mar = 27, 2018 at 12:01 AM, Joao De Almeida <= span id=3D"m_4680331108328681754m_-4863986252316515528m_-388912594589556856= 6:1tl.23">Pereira <jdealmeidapereira@p= ivotal.io> wrote:
Hello,
We tried to reproduce the issue but we= were not capable to reproduce it.=C2=A0
What it is strange on th= e fix is that python is complaining about a different line then the one tha= t was fixed. Maybe this is just a Python thing....
=

=C2=A0 =C2=A0 I have mentioned the steps = in RM and I have tried it on Ubuntu 16. Python is complai= ning the exact line where I have fixed the logic.=C2=A0
   ex_diag_message =3D u"{0}:  {1}".format(
=
     self.decode_to_utf8(excep=
tion_obj.diag.severity),         # exception_obj.diag.severity is not decoded before my fix.
     self.decode_to_utf8(exception_obj=
.diag.message_primary)
   )
=C2=A0=C2=A0

<= div>I assume that the fix works, but I would love to see some tests to ensu= re it is working. Another issue that looks more problematic is the fact tha= t, as per the Redmine issue, when an exception is thrown it sends back a Su= ccessful Query message. If this is the case then this fix doesn't look = like it is enough to solve the problem.

=C2=A0 =C2=A0 Yes it works. With the latest code I d= idn't see Successful Query message, it is showing "Not connected t= o the server .....", but the stack trace is same that was mentioned in= the RM.=C2=A0 =C2=A0=C2=A0

Thanks
Victoria & Joa= o

On Mon, Mar 26, 2018 at 9:00 AM Dave Page <dpage@pgadmin.org> wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px= #ccc solid;padding-left:1ex">
Thanks, applied.

On Mon, Mar 26, 2018 at 11:43 AM, Akshay Joshi = <aksh= ay.joshi@enterprisedb.com> wrote:
Hi Hackers,

Please find t= he attached patch to fix RM=C2=A0#3090 pgadmin shows misleading "= ;Query returned successfully" with incorrect SQL.

--
Akshay Joshi
Sr. Software Architect

=



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

EnterpriseDB UK: http://www.enterprisedb.com
The Enter= prise PostgreSQL Company



--
Aksh= ay Joshi
Sr. Software Architect

<= font color=3D"#3333FF">
=



--
Akshay= Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-824= 6
--94eb2c1252244ba4c90568725fd8--