From: | Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com> |
---|---|
To: | Dave Page <dave(dot)page(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 13:12:06 |
Message-ID: | CANxoLDeQZt-QxFkm3DO=9KaArySnfpysF8cZpojyWVd7ntTaxA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
Hi Hackers
Attached is the patch file to fix the RM #2815.
On Tue, Mar 20, 2018 at 3:24 PM, Dave Page <dave(dot)page(at)enterprisedb(dot)com>
wrote:
>
>
> 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
>
--
*Akshay Joshi*
*Sr. Software Architect *
*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
Attachment | Content-Type | Size |
---|---|---|
RM_2815.patch | application/octet-stream | 21.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Eckhardt | 2018-03-20 13:27:25 | Re: [pgadmin][patch] [GreenPlum] When user press Explain Plan and Explain analyze plan an error is displayed |
Previous Message | Dave Page | 2018-03-20 09:54:41 | Re: [pgadmin][patch] [GreenPlum] When user press Explain Plan and Explain analyze plan an error is displayed |