public inbox for [email protected]
help / color / mirror / Atom feedFrom: Akshay Joshi <[email protected]>
To: 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 15:18:44 +0530
Message-ID: <CANxoLDex35O8HVcNNT7-Tx+ug1fG8fYRn89Z0n6akYF25J2Z5g@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxowWOpZ9r+S2SvNgqncN6aFGpc4Rom7-S74LVEmP92Tdag@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>
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.
>
> 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-9517Mobile: +91 976-788-8246*
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]
Subject: Re: [pgadmin][patch] [GreenPlum] When user press Explain Plan and Explain analyze plan an error is displayed
In-Reply-To: <CANxoLDex35O8HVcNNT7-Tx+ug1fG8fYRn89Z0n6akYF25J2Z5g@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