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 1eyHIg-0005Y9-VK for pgadmin-hackers@arkaria.postgresql.org; Tue, 20 Mar 2018 13:27:39 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1eyHIf-0001YL-3p for pgadmin-hackers@arkaria.postgresql.org; Tue, 20 Mar 2018 13:27:37 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1eyHIe-0001YB-IX for pgadmin-hackers@lists.postgresql.org; Tue, 20 Mar 2018 13:27:36 +0000 Received: from mail-vk0-x243.google.com ([2607:f8b0:400c:c05::243]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1eyHIV-0006iJ-V5 for pgadmin-hackers@postgresql.org; Tue, 20 Mar 2018 13:27:34 +0000 Received: by mail-vk0-x243.google.com with SMTP id y127so902121vky.9 for ; Tue, 20 Mar 2018 06:27:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pivotal-io.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=w+KSJhUgGPQZQ86+vdJ6GLN0nRVseGtWlcmV9Nnn9z4=; b=OR1YPJDm1171sNg0m8Sq7iKYi6ygDdpBYhOawhDYfQEWQFDJPclzEy17rAINWGjbf7 XFy32wpb8ArvLTKJn9oDTq6Ayi5P1ccrnK07e3I144SkHMnIZFW6IhekoJNOgLazG7Je Nu84VC9v8/45HhHA2t84RLhauoNlKNhPjDexctFcAcSZy7q/+xuLozkcAYSXNO7AHspx FeY0OFX3ct2XN/vQA4dEykuZwwcTC2gbXymrCRpIBCxFyN8xHMIMpENx8CfmAeeRg+J3 iVxfVu5wQ6nFz1KJoufVzO/PapeS/l63ssOPaVwOd9PmJ+K6lZ/LkjzMJvpls23jTLJP b5AA== 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=w+KSJhUgGPQZQ86+vdJ6GLN0nRVseGtWlcmV9Nnn9z4=; b=FqaznQomQuQxgwpI96tONXK21YH1Hzy1oKL/X1PbHAaamWPm8dnKWRfmW0s50npKUe djJcpMXPGtDd2H6ADh4jxYIbMxMiKWCDlRL9hMtK4gJet889jTGdJBYWGDMIUXWFOzv8 gCUIcaZeIvfvfPmQ8ODk+6kBNRCeNEWNMSgE77+yQ/NOylt8Decvf2QkQtvOsNIDQ1lu GxBpTdax0bOdEC7xqR1wCUnPnga2R3kUGDhpR+ejzXhXBJnOmM01BNfXmYaJQQt7NOpl z2Ua8n8ubYXsYvKmUQ4T96oWqeBXs1rSsvwpd5wqoWvdAtBsmw+k0U3AVvwKafWAJAzQ PEQQ== X-Gm-Message-State: AElRT7GmcSy30kjo2IHniOy/cN4SO8YiWr/faLIwa2b2CExlSXHKAy24 L2TGTRHVqBIBuyqkpzoTs1w5p+e7lYJ4eD28lnaxAg== X-Google-Smtp-Source: AG47ELuU+jRB/+nEth6UhzX4oZJh9CKfoapTf5IMyc3IZ2g5Hpt/sLyZbVyxY5ZwD9CgMH0S+o5ibOBFGjIc6jhWUZg= X-Received: by 10.31.80.68 with SMTP id e65mr10929907vkb.182.1521552446030; Tue, 20 Mar 2018 06:27:26 -0700 (PDT) MIME-Version: 1.0 Received: by 10.176.72.111 with HTTP; Tue, 20 Mar 2018 06:27:25 -0700 (PDT) In-Reply-To: References: From: Robert Eckhardt Date: Tue, 20 Mar 2018 09:27:25 -0400 Message-ID: Subject: Re: [pgadmin][patch] [GreenPlum] When user press Explain Plan and Explain analyze plan an error is displayed To: Akshay Joshi Cc: Dave Page , Joao Pedro De Almeida Pereira , pgadmin-hackers Content-Type: multipart/alternative; boundary="001a114e2ec00d9f280567d80974" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --001a114e2ec00d9f280567d80974 Content-Type: text/plain; charset="UTF-8" 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 wrote: > Hi Hackers > > Attached is the patch file to fix the RM #2815. > > On Tue, Mar 20, 2018 at 3:24 PM, Dave Page > wrote: > >> >> >> On Tue, Mar 20, 2018 at 9:48 AM, Akshay Joshi < >> akshay.joshi@enterprisedb.com> wrote: >> >>> >>> >>> 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. >>> >> >> 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.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-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>* > --001a114e2ec00d9f280567d80974 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thanks for doing this, sorry about the breakage.=C2=A0
We're taking a look at this to make sure it is still wo= rking with Greenplum.=C2=A0

-- Rob

On Tue, Mar 20, 2018 = at 9:12 AM, Akshay Joshi <akshay.joshi@enterprisedb.com>= ; wrote:
Hi Hacke= rs

Attached is the patch file to fix the RM #2815.
=
On Tue, Mar 20, 2018 at 3:24 PM, Dave Page <dave.page@enterprisedb.com> wrote:


On Tue, Mar 20, 2018 at 9:48 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

<= div class=3D"m_7661474246643037949m_5282919280776697740m_428364908641225716= 3h5">
On Tue, Mar 20, 2018 at 7:51 AM, Akshay= Joshi <akshay.joshi@enterprisedb.com> wrot= e:
Hi Joao

It seems th= at 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.
Whil= e fixing the issue following problem that I found
  • In &quo= t;start_running_query.py" file, we need to remove check "= ;if conn.connecte= d()" from "__execute_query" function as we required exceptio= n to be thrown while executing the query to identify the ConnectionLost.=C2=A0=C2=A0<= /li>
  • In "execute_query.js" we have used axios to execute the query and in case of excepti= on, object is different then normal javascript response object.=C2=A0
    • <= li>We call following functions when exception or error comes and send the &= quot;<object>.response.data" as parameter=C2=A0
      <= li>wasConnectionLostToSe= rver(): Check for the readyState parameter, which is not the part of "<object= >.response.data".=C2=A0
    • extractErrorMessage(): Check for the "respon= seJSON" and "responseJSON.info",=C2=A0which is not the part of "<object>.response.data".<= /li>
    • is_pga_<= span id=3D"m_7661474246643037949m_5282919280776697740m_4283649086412257163m= _-7457526324914864622m_4323628897702125467:2bc.13">login_required():= Check for the=C2=A0&quo= t;responseJSON&q= uot; and "response= JSON.info",=C2=A0which i= s not the part of "<object>.response.data".
    • = is_new_transaction_required():=C2=A0Check for= the=C2=A0"<= span id=3D"m_7661474246643037949m_5282919280776697740m_4283649086412257163m= _-7457526324914864622m_4323628897702125467:2bc.16">responseJSON"= ; and "responseJSO= N.info",=C2=A0which is n= ot the part of "<object>.response.data".
    From the above list, some of the function calls are generic where they n= eed "responseJSON<= /span>" and "= responseJSON.info", so we can't change that. Possible solut= ion could be pass one extra flag as parameter to identify the object is a <= span id=3D"m_7661474246643037949m_5282919280776697740m_4283649086412257163m= _-7457526324914864622m_4323628897702125467:2bc.20">axios response or= javascript response to above functions and change the logic accordingly.

    Please let me know your thoughts or any other sugge= stion.=C2=A0
    =C2=A0 =C2=A0 =C2=A0=C2=A0

On Fri, Feb 9, 2018 at 8:17 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks, app= lied.

On Fri, Feb 9, 2018 at 2:35 PM, Joao De Almeida Perei= ra <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 <dpage@pgadmin.org>= ; wrote:
Hi Joao,<= div>
It looks like Jenkins has taken umbrage to this change, = at least with Python 3.x. Can you take a look please?

<= div>https://jenk= ins.pgadmin.org/

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 Pereira= <jdealmeidapereira@pivotal.io> wrote:
Hi Hackers,
This is quite a bi= g patch in order to solve the problem with the Explain Plan.

=
We sent 2 patches that have the following:
-=C2=A0u= pdate-javascript-packages.diff=C2=A0
=C2=A0 =C2=A0 = Add package:
=C2=A0 =C2=A0 =C2=A0is-docker to select a specif= ic setting when running the Chrome tests in
=C2=A0 =C2=A0 =C2=A0D= ocker

=C2=A0 =C2=A0 Upgrade the version of:
<= div>=C2=A0 =C2=A0 - babel-loader
=C2=A0 =C2=A0 - extract-text-web= pack-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.execu= te and SQLEditor._poll into their own files and add test around them
<= div>=C2=A0 Extract SQLEditor backend functions that start executing query t= o their own files and add tests around it
=C2=A0 Move the Explain= 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_templa= te_name into a function that can be used by the different places that try t= o 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.e= nterprisedb.com
The Enterprise PostgreSQL Company



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitt= er: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postg= reSQL Company



<= /div>--
Akshay Joshi
=
Sr. Software Arc= hitect





--
Dave Page
VP, Chief Architect, Tools & Installers
Enter= priseDB: http://w= ww.enterprisedb.com
The Enterprise PostgreSQL Company

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



--
Akshay Joshi
Sr. Software Architect

<= font color=3D"#3333FF">
=
=


--
Dave Page
VP, Chief Architect, Tools & Installers
EnterpriseDB:= http://www.enter= prisedb.com
The Enterprise PostgreSQL Company

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



--
Akshay Joshi
Sr. Software Architect

=
Phone: +91 20-3058-9517
Mobile: +91 976-788-8246

--001a114e2ec00d9f280567d80974--