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

From: Robert Eckhardt <reckhardt(at)pivotal(dot)io>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: Dave Page <dave(dot)page(at)enterprisedb(dot)com>, 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:27:25
Message-ID: CAAtBm9UM7jQs1BdVGPvvauvj87ZDnxzaAa7uBzCHby7bEK90cw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks for doing this, sorry about the breakage.

We're taking a look at this to make sure it is still working with
Greenplum.

-- Rob

On Tue, Mar 20, 2018 at 9:12 AM, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com
> wrote:

> 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-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
> <+91%2097678%2088246>*
>

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2018-03-20 13:48:42 pgAdmin 4 commit: 3.0 release notes
Previous 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