Re: [pgadmin][patch] [GreenPlum] When user press Explain Plan and Explain analyze plan an error is displayed

On Tue, Mar 20, 2018 at 3:06 PM, Dave Page <dave(dot)page(at)enterprisedb(dot)com>
wrote:

> I'm a little concerned that noone mentioned this earlier; I'm supposed to
> be building the release this afternoon, and I expect this change to at the
> very least be complex to fully test and verify. What's the ETA on the
> patch? What steps are being taken to ensure it's correct and doesn't cause
> regressions?
>

Harshal has already mentioned in the RM. Currently I am changing the
logic, but it may take time to complete, fully test and verify. I'll try my
best to do it asap.

>
> On Tue, Mar 20, 2018 at 7:51 AM, Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi Joao
>>
>> It seems that this fix broke the functionality of RM #2815. It is
>> mentioned in the RM what needs to be fixed now and I am currently working
>> on it.
>> While fixing the issue following problem that I found
>>
>> - In "start_running_query.py" file, we need to remove check "if conn.connected()"
>> from "__execute_query" function as we required exception to be thrown while
>> executing the query to identify the ConnectionLost.
>> - In "execute_query.js" we have used *axios* to execute the query and
>> in case of exception, object is different then normal javascript response
>> object.
>> - We call following functions when exception or error comes and send
>> the "*<object>.response.data*" as parameter
>> - wasConnectionLostToServer(): Check for the readyState parameter,
>> which is not the part of "<object>.response.data".
>> - extractErrorMessage(): Check for the "responseJSON" and "
>> responseJSON.info", which is not the part of
>> "<object>.response.data".
>> - is_pga_login_required(): Check for the "responseJSON" and "
>> responseJSON.info", which is not the part of
>> "<object>.response.data".
>> - is_new_transaction_required(): Check for the "responseJSON" and "
>> responseJSON.info", which is not the part of
>> "<object>.response.data".
>>
>> From the above list, some of the function calls are generic where they
>> need "responseJSON" and "responseJSON.info", so we can't change that.
>> Possible solution could be pass one extra flag as parameter to identify the
>> object is a axios response or javascript response to above functions and
>> change the logic accordingly.
>>
>> Please let me know your thoughts or any other suggestion.
>>
>>
>> On Fri, Feb 9, 2018 at 8:17 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Thanks, applied.
>>>
>>> On Fri, Feb 9, 2018 at 2:35 PM, Joao De Almeida Pereira <
>>> jdealmeidapereira(at)pivotal(dot)io> wrote:
>>>
>>>> Hello,
>>>> Attached you can find the fix for the current pronlem
>>>>
>>>>
>>>> On Fri, Feb 9, 2018 at 7:29 AM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>> Hi Joao,
>>>>>
>>>>> It looks like Jenkins has taken umbrage to this change, at least with
>>>>> Python 3.x. Can you take a look please?
>>>>>
>>>>> https://jenkins.pgadmin.org/
>>>>>
>>>>> Thanks.
>>>>>
>>>>> On Fri, Feb 9, 2018 at 11:54 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>> Thanks, patches applied.
>>>>>>
>>>>>> On Fri, Feb 2, 2018 at 10:50 PM, Joao De Almeida Pereira <
>>>>>> jdealmeidapereira(at)pivotal(dot)io> wrote:
>>>>>>
>>>>>>> Hi Hackers,
>>>>>>> This is quite a big patch in order to solve the problem with the
>>>>>>> Explain Plan.
>>>>>>>
>>>>>>> We sent 2 patches that have the following:
>>>>>>> *- update-javascript-packages.diff *
>>>>>>> Add package:
>>>>>>> is-docker to select a specific setting when running the Chrome
>>>>>>> tests in
>>>>>>> Docker
>>>>>>>
>>>>>>> Upgrade the version of:
>>>>>>> - babel-loader
>>>>>>> - extract-text-webpack-plugin
>>>>>>> - jasmine-core
>>>>>>> - jasmine-enzyme
>>>>>>> - moment
>>>>>>> *- explain-plan-greenplum.diff*
>>>>>>> Extract SQLEditor.execute and SQLEditor._poll into their own files
>>>>>>> and add test around them
>>>>>>> Extract SQLEditor backend functions that start executing query to
>>>>>>> their own files and add tests around it
>>>>>>> Move the Explain SQL from the front-end and now pass the Explain
>>>>>>> plan parameters as a JSON object in the start query call.
>>>>>>> Extract the compile_template_name into a function that can be used
>>>>>>> by the different places that try to select the version of the template and
>>>>>>> the server type
>>>>>>>
>>>>>>>
>>>>>>> Thanks
>>>>>>> Joao
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> 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
>>>
>>
>>
>>
>> --
>> *Akshay Joshi*
>>
>> *Sr. Software Architect *
>>
>>
>>
>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
>> <+91%2097678%2088246>*
>>
>
>
>
> --
> Dave Page
> VP, Chief Architect, Tools & Installers
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>

--
*Akshay Joshi*

*Sr. Software Architect *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*