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 1eyDym-0002PK-Ky for pgadmin-hackers@arkaria.postgresql.org; Tue, 20 Mar 2018 09:54:52 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1eyDyk-0007Oq-Fb for pgadmin-hackers@arkaria.postgresql.org; Tue, 20 Mar 2018 09:54:50 +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 1eyDyk-0007OJ-1V for pgadmin-hackers@lists.postgresql.org; Tue, 20 Mar 2018 09:54:50 +0000 Received: from mail-wm0-x234.google.com ([2a00:1450:400c:c09::234]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1eyDyg-0001st-8L for pgadmin-hackers@postgresql.org; Tue, 20 Mar 2018 09:54:48 +0000 Received: by mail-wm0-x234.google.com with SMTP id e194so2116956wmd.3 for ; Tue, 20 Mar 2018 02:54:45 -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=IdAhEKZsena2FB411OEk+SdY27IkSJV9y9s0j3bgAC4=; b=hPxlYw+Vjn9OWqDOxlVc8l79uEo6p0eGRsGBtcrGQu/3LuE7lm7vx2QapORC8zFgI/ tj2sJej8UwzXPtvkrrEPIn6APujcF5IFbyu417iw66Q7VECr57SKO7lSTKKsSlmb5j+h XVgNnJ9gltwE5/WZctnW8RBZJ92xvHS2v0Sn7zeoZDMBSeJt8Xka+j763IW24w4bfkUM bT8SiQIu45ftoc5P7ghLSlwGzUAubET47eB/N/dQoknj8Gu6Qyjerb94uoY0j20jqDQW Nf2FXKktuR+p5uHvEuaqNv62wxvG76fdD986mNxwNJB4TKrEPozvH52oFosTzUIQVeqM CLzg== 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=IdAhEKZsena2FB411OEk+SdY27IkSJV9y9s0j3bgAC4=; b=lZzRl32pbJF/oQEZkAoLDEiVPZEsDqU9tfz+QQdloyn2JU1YtcvJ5ZvML8QGpK4hfS wtX9aj9WKQkz9YPUbVNP0G8CTj4vAkEdXFGlRXa6dQaCQAuljCHsZIQaoi8NC0IU+fvz 7+lnZGVv/cIxps4JO8TzG8qJ2lukzHDCnzhYTj2krtdkgHUNKh2mT4aMcfknLfCbdGHp fd5RTPSzajiF+CspZc+pp8GpT3ZeCl1YMft+3JKmnf163d7Ub8NwAzqmrrjca/ioKwpB vqVPYMqu1aNmqjuHDs0S1xKCujp3r+u9orxtFqLt+H1/H+zrHtrbi3oG42bNJIC2vij8 Tqyw== X-Gm-Message-State: AElRT7EWt2c+gNok8uCnBbbnN+ngVim7EWXNKaFMGe8WS3SJ/EUi8vYX 4hQfppug72rHMbicfmNVEYlHExPF8VMBEhgPFYWb2kqRZB31CH6pPCjvcDhCVZOX7g3oj27W5Uh Q7DjL6jQTyPBcOB66p2dP9szORW5OdkpvnFNvWKQqPceBvque37HzyDUrcKlYW9fMYnbuOSRVe7 XgIklxjg== X-Google-Smtp-Source: AG47ELssMi/dpVZJ08vBiX3zCgQ+Z06FLtu6qmxHPetBh/AITWdQ38nhte28gi31ZPrqiACx+1w2og== X-Received: by 10.80.138.142 with SMTP id j14mr12942877edj.36.1521539684230; Tue, 20 Mar 2018 02:54:44 -0700 (PDT) Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com. [74.125.82.44]) by smtp.gmail.com with ESMTPSA id 93sm1546499edi.19.2018.03.20.02.54.42 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 20 Mar 2018 02:54:43 -0700 (PDT) Received: by mail-wm0-f44.google.com with SMTP id e194so2116746wmd.3 for ; Tue, 20 Mar 2018 02:54:42 -0700 (PDT) X-Received: by 10.28.98.69 with SMTP id w66mr1710473wmb.64.1521539682510; Tue, 20 Mar 2018 02:54:42 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.69.220 with HTTP; Tue, 20 Mar 2018 02:54:41 -0700 (PDT) In-Reply-To: References: From: Dave Page Date: Tue, 20 Mar 2018 09:54:41 +0000 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [pgadmin][patch] [GreenPlum] When user press Explain Plan and Explain analyze plan an error is displayed To: Akshay Joshi Cc: Joao Pedro De Almeida Pereira , pgadmin-hackers Content-Type: multipart/alternative; boundary="001a1148e3d249c5c70567d510f1" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --001a1148e3d249c5c70567d510f1 Content-Type: text/plain; charset="UTF-8" On Tue, Mar 20, 2018 at 9:48 AM, Akshay Joshi 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 --001a1148e3d249c5c70567d510f1 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


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


On Tue, Mar 20, 2018 at 3:06 PM, Dave Page <dave.page@enterprisedb.com> wrote:
=
I'm a little concerned = that noone mentioned this earlier; I'm supposed to be building the rele= ase this afternoon, and I expect this change to at the very least be comple= x 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?<= /div>

=C2=A0 =C2=A0 Harshal has already mentioned in the RM. Cu= rrently I am changing the logic, but it may take time to complete, fully te= st and verify. I'll try my best to do it asap.
<= /blockquote>

Sure, but how many of us are watching every= comment on every RM? I know I'm not (I currently average ~400 emails/d= ay).
=C2=A0
=

On Tue, Mar 20,= 2018 at 7:51 AM, Akshay Joshi <akshay.joshi@enterprisedb.com<= /a>> wrote:
Hi Joao

It seems that this fix broke = the functionality of RM #2815. It is mentioned in the RM what needs to be f= ixed 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 rem= ove check "if conn.connected()" from "__exec= ute_query" function as we required exception to be thrown while execut= ing the query to identify the ConnectionLost.=C2=A0=C2=A0
  • 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.=C2=A0
  • = We call following functions when exception or error comes and send the &quo= t;<object>.response.data" as parameter=C2=A0
    • = wasConnectionLostToServer(): Check for the readySt= ate parameter, which is not the part of "<object>.respons= e.data".=C2=A0
    • extractErrorMessage(): Check fo= r the "responseJSON" a= nd "responseJSON.info",=C2=A0which is not the part of "<object>.response.data&q= uot;.
    • is_pga_login_require= d(): Check for the=C2=A0= "responseJSON" and "responseJSON<= /span>.info",=C2=A0which is not= the part of "<object>.response.data".
    • is_ne= w_transaction_required():=C2=A0Check for the= =C2=A0"responseJSON" and "responseJSON.info= ",=C2=A0which is not the part o= f "<object>.response.data".
From the a= bove list, some of the function calls are generic where they need "responseJSON" and "responseJSON.in= fo", 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 acc= ordingly.

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

On Fri, Feb 9, 2018 at 8:17 P= M, Dave Page <dpage@pgadmin.org> 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 <dpage@pgadmin.org> wrote:<= br>
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?


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 P= M, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io>= ; wrote:
Hi Hacke= rs,
This is quite a big patch in order to solve the problem with the Ex= plain Plan.

We sent 2 patches that have the follow= ing:
-=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 Upgr= ade the version of:
=C2=A0 =C2=A0 - babel-loader
=C2=A0= =C2=A0 - extract-text-webpack-plugin
=C2=A0 =C2=A0 - jasmine-cor= e
=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 query to their own files and add tests around it
=C2=A0 Move the Explain SQL from the front-end and now pass the Explain pl= an parameters as a JSON object in the start query call.
=C2=A0 Ex= tract the compile_template_name into a function that can be used by the dif= ferent places that try to select the version of the template and the server= type


Thanks
Joao



-= -
Dave= Page
Blog: ht= tp://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: = http://www.enterp= risedb.com
The Enterprise PostgreSQL Company



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twit= ter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Post= greSQL Company



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

EnterpriseDB UK: http://www.enterprised= b.com
The Enterprise PostgreSQL Company



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

=

<= /font>



--
=
Dave Page
VP, Chief Architect, Tools &= amp; Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQ= L Company

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



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

<= /font>

<= /div>



--
Dave Page
VP, Ch= ief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
Th= e Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pg= snake
--001a1148e3d249c5c70567d510f1--