Re: [pgAdmin4][patch]: RM #3090 pgadmin shows misleading "Query returned successfully" with incorrect SQL

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.

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.

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 <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> Hi Joao
>
> On Tue, Mar 27, 2018 at 12:01 AM, Joao De Almeida Pereira <
> jdealmeidapereira(at)pivotal(dot)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 <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Thanks, applied.
>>>
>>> On Mon, Mar 26, 2018 at 11:43 AM, Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)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>*
>