Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1eyDsz-00028U-3Z for pgadmin-hackers@arkaria.postgresql.org; Tue, 20 Mar 2018 09:48:53 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1eyDsx-0006Mw-Nx for pgadmin-hackers@arkaria.postgresql.org; Tue, 20 Mar 2018 09:48:51 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1eyDsx-0006Mm-HT for pgadmin-hackers@lists.postgresql.org; Tue, 20 Mar 2018 09:48:51 +0000 Received: from mail-qt0-x241.google.com ([2607:f8b0:400d:c0d::241]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1eyDst-0001H9-En for pgadmin-hackers@postgresql.org; Tue, 20 Mar 2018 09:48:50 +0000 Received: by mail-qt0-x241.google.com with SMTP id n11so592368qti.4 for ; Tue, 20 Mar 2018 02:48:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=9WWJh2GX7ppjVHtsfEIE8FoBgSFh59ZneBMdThdRLy8=; b=pcjmOKSrgemhtbJjTvmloZ/5XePqzkyABh9E/FASCx/bzCy4BavWyrTZU+8dZXQHOB eyae415TQHoEP4MUN8rxxuJxh/D4C+6exT2Rf41SYwtodQtlUyEEb6E/feyKVC1r0xGE 8xYSagnQoV1w3ZVgopzktrmbjt6Ej/0PRa4A4F5m/fZ2/DqSZ8qvf4pNrtcyH8i9LI7M cSy7azJ4A62O+9hhYd0zR4D9SfwbD57Y3QPN0EPgPApYceJzsuB7khZ0CgE8CtuaioaH II9/a/9Lz+2860EX8zetnhLuWA/KhfxslkXRegLDvd2ewkMkD0f7oYZdRXbt6UKun2f+ X06w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=9WWJh2GX7ppjVHtsfEIE8FoBgSFh59ZneBMdThdRLy8=; b=djt6CawhT/ZXPR3b0Zc9d/2G/GKvCOWhCiyjseQklKhczxsfCq1C9PmBCWx6yEEUj2 2lfzfDbr+wxPFvR0eo0BwOQghRxdh4s1gGwRUHhlEpF57QUl3bo/9X8tFnqtoV+wKpws Fbtx9PXMJedsSGSX+BMKI8GnlpuY9XlWOAT12RoZIab9cpa+cMoFha8qk4oo3itstZp2 seju5JzE6c70MVs6HjrdoQ30J4H+/pax+r0qyyFbWLwPTgaB3VCeKuVXH3g5dlCZ8hVh P/LBRLNUmBTkg9zmjRlL7QFoAqvvBGn+wPippqOPMJ0q7NzhOPp6YAAW1rTsU9auR+lW FeKw== X-Gm-Message-State: AElRT7GYvkffhQRIx5kUrkTF41Qq2HYY6c2G9zyVJdHwFOQQo5ZaHr0X VDN1KYNGA72SS75mMcY9JJowRbUnLTk86Jgt8Wvv/Q== X-Google-Smtp-Source: AG47ELu3LY7q1V7mJ/oCQKeKjcP6ksQlrdoYa/Sm0aMyyG6HA3gYGWIUztZyj0YrsC0TyI46bB3TxsDWxaNFkEuoz2w= X-Received: by 10.237.50.227 with SMTP id z90mr23292678qtd.126.1521539325265; Tue, 20 Mar 2018 02:48:45 -0700 (PDT) MIME-Version: 1.0 Received: by 10.12.194.131 with HTTP; Tue, 20 Mar 2018 02:48:44 -0700 (PDT) In-Reply-To: References: From: Akshay Joshi Date: Tue, 20 Mar 2018 15:18:44 +0530 Message-ID: Subject: Re: [pgadmin][patch] [GreenPlum] When user press Explain Plan and Explain analyze plan an error is displayed To: Dave Page Cc: Joao Pedro De Almeida Pereira , pgadmin-hackers Content-Type: multipart/alternative; boundary="94eb2c12555afe9c480567d4fa35" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --94eb2c12555afe9c480567d4fa35 Content-Type: text/plain; charset="UTF-8" On Tue, Mar 20, 2018 at 3:06 PM, Dave Page 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 < > akshay.joshi@enterprisedb.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 "*.response.data*" as parameter >> - wasConnectionLostToServer(): Check for the readyState parameter, >> which is not the part of ".response.data". >> - extractErrorMessage(): Check for the "responseJSON" and " >> responseJSON.info", which is not the part of >> ".response.data". >> - is_pga_login_required(): Check for the "responseJSON" and " >> responseJSON.info", which is not the part of >> ".response.data". >> - is_new_transaction_required(): Check for the "responseJSON" and " >> responseJSON.info", which is not the part of >> ".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 wrote: >> >>> Thanks, applied. >>> >>> On Fri, Feb 9, 2018 at 2:35 PM, Joao De Almeida Pereira < >>> jdealmeidapereira@pivotal.io> wrote: >>> >>>> Hello, >>>> Attached you can find the fix for the current pronlem >>>> >>>> >>>> On Fri, Feb 9, 2018 at 7:29 AM Dave Page 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 wrote: >>>>> >>>>>> Thanks, patches applied. >>>>>> >>>>>> On Fri, Feb 2, 2018 at 10:50 PM, Joao De Almeida Pereira < >>>>>> jdealmeidapereira@pivotal.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-9517Mobile: +91 976-788-8246* --94eb2c12555afe9c480567d4fa35 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Tue, Mar 20, 2018 at 3:06 PM, Dave Page <dave.page@enterprisedb.com> wrote:
I'm a li= ttle concerned that noone mentioned this earlier; I'm supposed to be bu= ilding 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 caus= e regressions?

=C2=A0 =C2=A0 Harshal has alread= y mentioned in the RM. Currently I am changing the logic, but it may take t= ime 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 <akshay.joshi@enterprisedb.com> wrote:
Hi Joao

It seems that this fix broke the functionality of RM #2815. It is m= entioned 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
<= div>
  • In "start_running_query.py" file= , we need to remove check "if conn.connected()" from "__execute_q= uery" function as we required exception to be thrown while executing t= he query to identify the ConnectionLost.=C2=A0=C2=A0
  • In "exe= cute_query.j= s" we have used axios to execute the query and in case of exc= eption, object is different then normal javascript response object.=C2=A0
  • We call following functions when exception or error comes and send t= he "<object>.response.data" as parameter=C2=A0
    • <= ul>
  • wasCo= nnectionLostToServer(): Check for the readyState parameter, which is not = the part of "<object>.response.data".=C2=A0
  • extractErrorMessage= (): Check for the "responseJSON" and= "resp= onseJSON.info",=C2=A0which is n= ot the part of "<object>.response.data".
  • is_= pga_= login_required(): Check for the=C2=A0"responseJSON" and "responseJSON.info",=C2=A0which is not the part of "<object>= .response.data".
  • is_new_transaction_required():=C2= =A0Check for the=C2=A0"responseJSON" and "responseJSON.info&= quot;,=C2=A0which is not the part of= "<object>.response.data".
From the ab= ove list, some of the function calls are generic where they need "responseJSON" and "responseJSON.info", so we can't change that. Poss= ible solution could be pass one extra flag as parameter to identify the obj= ect is a ax= ios response or javascript response to above functions and change th= e logic accordingly.

Please let me know your thoug= hts or any other suggestion.=C2=A0
=C2=A0 =C2=A0 =C2=A0=C2=A0

On Fri, Feb 9, 2018 at 8:17 PM, Dave Pa= ge <dpage@pgadmin.org> wrote:
Thanks, applied.

On Fri, Feb 9, 2018 at 2:35 PM, Joao De Almeida Pereira <jdea= lmeidapereira@pivotal.io> wrote:
Hello,
Attached you can find the fix for the cur= rent pronlem


On Fri, Feb 9, 2018 at 7:29 AM Dave Page <dpage@pgadmin.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?


Thanks.

On Fri, Feb 9, 2018 at 11:54 AM= , Dave Page <dpage@pgadmin.org> wrote:
Thanks, patches applied.

On Fri, Feb 2, 2018 at 10:50 PM, Joao De Almeida Pe= reira <jdealmeidapereira@pivotal.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:
-= =C2=A0update-javascript-packages.diff=C2=A0
=C2=A0 = =C2=A0 Add package:
=C2=A0 =C2=A0 =C2=A0is-docker to select a= specific setting when running the Chrome tests in
=C2=A0 =C2=A0 = =C2=A0Docker

=C2=A0 =C2=A0 Upgrade the version of:=
=C2=A0 =C2=A0 - babel-loader
=C2=A0 =C2=A0 - extract-t= ext-webpack-plugin
=C2=A0 =C2=A0 - jasmine-core
=C2=A0 = =C2=A0 - jasmine-enzyme
=C2=A0 =C2=A0 - moment
-=C2=A0explain-plan-greenplum.diff
=C2=A0 Extract SQLEditor.= execute and SQLEditor._poll into their own files and add test around them
=C2=A0 Extract SQLEditor backend functions that start executing qu= ery to their own files and add tests around it
=C2=A0 Move the Ex= plain SQL from the front-end and now pass the Explain plan parameters as a = JSON object in the start query call.
=C2=A0 Extract the compile_t= emplate_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<= /a>
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Ente= rprise PostgreSQL Company



<= /div>-= -
Akshay Joshi=
Sr. Software Architect


<= b>Phone: +91 20-3058-9517
Mobile: <= a href=3D"tel:+91%2097678%2088246" value=3D"+919767888246" target=3D"_blank= ">+91 976-788-8246



--
<= span class=3D"HOEnZb">Dave Page
VP, Chief Archit= ect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterpri= se PostgreSQL Company

Blog: http://pgsnake.blogspot.= com
Twitter: @pgsnake



--
Akshay= Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-824= 6
--94eb2c12555afe9c480567d4fa35--