From: | Dave Page <dave(dot)page(at)enterprisedb(dot)com> |
---|---|
To: | Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com> |
Cc: | Joao Pedro De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: [pgadmin][patch] [GreenPlum] When user press Explain Plan and Explain analyze plan an error is displayed |
Date: | 2018-03-20 09:54:41 |
Message-ID: | CA+OCxoz9Bj8=q6440-sK=x_10kCquT1fNfpFimysrBRZwgq_Bw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
On Tue, Mar 20, 2018 at 9:48 AM, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com
> wrote:
>
>
> 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.
>
Sure, but how many of us are watching every comment on every RM? I know I'm
not (I currently average ~400 emails/day).
>
>> 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-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
From | Date | Subject | |
---|---|---|---|
Next Message | Akshay Joshi | 2018-03-20 13:12:06 | Re: [pgadmin][patch] [GreenPlum] When user press Explain Plan and Explain analyze plan an error is displayed |
Previous Message | Akshay Joshi | 2018-03-20 09:48:44 | Re: [pgadmin][patch] [GreenPlum] When user press Explain Plan and Explain analyze plan an error is displayed |