public inbox for [email protected]
help / color / mirror / Atom feedFrom: Robert Eckhardt <[email protected]>
To: Akshay Joshi <[email protected]>
Cc: Dave Page <[email protected]>
Cc: Joao Pedro De Almeida Pereira <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgadmin][patch] [GreenPlum] When user press Explain Plan and Explain analyze plan an error is displayed
Date: Tue, 20 Mar 2018 09:27:25 -0400
Message-ID: <CAAtBm9UM7jQs1BdVGPvvauvj87ZDnxzaAa7uBzCHby7bEK90cw@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDeQZt-QxFkm3DO=9KaArySnfpysF8cZpojyWVd7ntTaxA@mail.gmail.com>
References: <CAE+jjakOhMJBMqsUaQuL9w4an0V4Q4nvY51K3GqWzm8jbj9cog@mail.gmail.com>
<CA+OCxowBzhOaJf1_w1gPhOR4TH3bdtKBsdqETAqSwK0n2GWezg@mail.gmail.com>
<CA+OCxoyT9yQU-ayjRa94UMaekGP7QhB3CNqaJ_A-j8t+4Jb4FA@mail.gmail.com>
<CAE+jjam6bc4HeZgFbRayxXOJVft6J7s=Xb7M8i54mX7tNCp08Q@mail.gmail.com>
<CA+OCxozK4FeKyugzWNc2aRC1cMONiQMbFgus7Aj=7U28CaKV=Q@mail.gmail.com>
<CANxoLDecQruspbc0dyw6ARNAM6DYJF5pC_0UJPqUzAM9spQHYg@mail.gmail.com>
<CA+OCxowWOpZ9r+S2SvNgqncN6aFGpc4Rom7-S74LVEmP92Tdag@mail.gmail.com>
<CANxoLDex35O8HVcNNT7-Tx+ug1fG8fYRn89Z0n6akYF25J2Z5g@mail.gmail.com>
<CA+OCxoz9Bj8=q6440-sK=x_10kCquT1fNfpFimysrBRZwgq_Bw@mail.gmail.com>
<CANxoLDeQZt-QxFkm3DO=9KaArySnfpysF8cZpojyWVd7ntTaxA@mail.gmail.com>
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 <[email protected]
> wrote:
> Hi Hackers
>
> Attached is the patch file to fix the RM #2815.
>
> On Tue, Mar 20, 2018 at 3:24 PM, Dave Page <[email protected]>
> wrote:
>
>>
>>
>> On Tue, Mar 20, 2018 at 9:48 AM, Akshay Joshi <
>> [email protected]> wrote:
>>
>>>
>>>
>>> On Tue, Mar 20, 2018 at 3:06 PM, Dave Page <[email protected]>
>>> 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 <
>>>> [email protected]> 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 <[email protected]> wrote:
>>>>>
>>>>>> Thanks, applied.
>>>>>>
>>>>>> On Fri, Feb 9, 2018 at 2:35 PM, Joao De Almeida Pereira <
>>>>>> [email protected]> wrote:
>>>>>>
>>>>>>> Hello,
>>>>>>> Attached you can find the fix for the current pronlem
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Feb 9, 2018 at 7:29 AM Dave Page <[email protected]> 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 <[email protected]>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Thanks, patches applied.
>>>>>>>>>
>>>>>>>>> On Fri, Feb 2, 2018 at 10:50 PM, Joao De Almeida Pereira <
>>>>>>>>> [email protected]> 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>*
>
view thread (19+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: [pgadmin][patch] [GreenPlum] When user press Explain Plan and Explain analyze plan an error is displayed
In-Reply-To: <CAAtBm9UM7jQs1BdVGPvvauvj87ZDnxzaAa7uBzCHby7bEK90cw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox