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 1eyDgz-0001Xy-BO for pgadmin-hackers@arkaria.postgresql.org; Tue, 20 Mar 2018 09:36:29 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1eyDgx-0002vv-Mz for pgadmin-hackers@arkaria.postgresql.org; Tue, 20 Mar 2018 09:36:27 +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 1eyDgx-0002vl-Cd for pgadmin-hackers@lists.postgresql.org; Tue, 20 Mar 2018 09:36:27 +0000 Received: from mail-wm0-x232.google.com ([2a00:1450:400c:c09::232]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1eyDgt-0000zv-Cn for pgadmin-hackers@postgresql.org; Tue, 20 Mar 2018 09:36:26 +0000 Received: by mail-wm0-x232.google.com with SMTP id f19so2050595wmc.0 for ; Tue, 20 Mar 2018 02:36:22 -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=oQ57j9g7+Ozpycr2nieGJhoMJQz/ANFDEW9FqicdJPM=; b=nQM9REkVLORgC2u4ODsR+zfC8QBtcQT7y9m2XJoHKLEaO77OAUT90MAKj9aeEqTlZH z0+YZXMmdZ7uDGd7fGOhv0M5F6IwaP6XSnBVlb7O4+I3qCDF/Ul0CLEeAHZr8sLEFgOm JLeWmSUFpKyD5gU+MnNlZnnm4C3JBSsbCC4bD083AGbbAnvVIVr81vGTe/dusInXgB5v BbuNBy92+t3Zlk+PvM/WlJvirKiqiwZRgAJ87qf9RhkTSpuId6czWC/oD0pdpDTZ4cMm s3UgVWg3iFYO7uPn+5JxF6XnaPKUK7WwQ3KePkEJJhn5s1VEFXT4BP5Sq9YhsUQVPVKR TTsg== 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=oQ57j9g7+Ozpycr2nieGJhoMJQz/ANFDEW9FqicdJPM=; b=oqkiAXR6UHNtjOIESAzyY7Zc2g4Rm6a+x/ZPUD3XXxHpFpgv9boVF4izoy8m8jL/KR +BPdTKNSYqozR+NmW2IfwOpkcNc35pN3uEuejdPlUHZ4KXO53kTjCUP8Smm4uyAGds4d zZHcx0NS/m/EoLMsOUZzYWEHIXR4PP6JLueHvaALgTKJyaPa8TR33LrGaV6FzcbivPAk NdhIukTvmWx9FKTGUZjGrWWN8yY4Q2FYIRbdV71ItIBkw3I8xYfH+ooOfU7r+KUl6Vcj ORg710pmaSZvsadmahTmvSd5bOx608FWrdyohTvlp3KH0gBdgiOWHNcvDfrxzrcd5BiZ y/Yg== X-Gm-Message-State: AElRT7GFbyDmBv8TisK6Fhsua6Dci43rCCGMrx+0EYgkhA8H1WvDJPMb rb2avRuSPnrr7//lXjTxYbr8Aa8c5ostSiFC30bbI+ZqTc/HfX3+pDKUnDsWBVSAsuoBEU4jmej zGdn5R29nQytUrAHRgMKQ9D98/ypPegnN798smluJnu8sYsiy95eoYYiL5dhNeYm7D6cH3e7SCK +7FR7umA== X-Google-Smtp-Source: AG47ELs7ZzR+9H3EtiFjxIFloevQpu/WmnGuP6PeE2UY1nfRIz1goLBNXtm1ceGdmuC5FecLMqsYmA== X-Received: by 10.80.166.211 with SMTP id f19mr17030281edc.266.1521538581988; Tue, 20 Mar 2018 02:36:21 -0700 (PDT) Received: from mail-wr0-f181.google.com (mail-wr0-f181.google.com. [209.85.128.181]) by smtp.gmail.com with ESMTPSA id j42sm81447eda.67.2018.03.20.02.36.20 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 20 Mar 2018 02:36:20 -0700 (PDT) Received: by mail-wr0-f181.google.com with SMTP id u46so926764wrc.11 for ; Tue, 20 Mar 2018 02:36:20 -0700 (PDT) X-Received: by 10.223.176.98 with SMTP id g31mr11889660wra.256.1521538580266; Tue, 20 Mar 2018 02:36:20 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.69.220 with HTTP; Tue, 20 Mar 2018 02:36:19 -0700 (PDT) In-Reply-To: References: From: Dave Page Date: Tue, 20 Mar 2018 09:36:19 +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="001a113c9c4096db510567d4ce37" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --001a113c9c4096db510567d4ce37 Content-Type: text/plain; charset="UTF-8" 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? On Tue, Mar 20, 2018 at 7:51 AM, Akshay Joshi 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 --001a113c9c4096db510567d4ce37 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
I'm a little concerned that noone mentioned this earli= er; I'm supposed to be building the release this afternoon, and I expec= t this change to at the very least be complex to fully test and verify. Wha= t's the ETA on the patch? What steps are being taken to ensure it's= correct and doesn't cause regressions?

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

It seems that this fix broke t= he functionality of RM #2815. It is mentioned in the RM what needs to be fi= xed 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&qu= ot; function as we required exception to be thrown while executing the quer= y to identify the ConnectionLost.=C2=A0=C2=A0
  • In "execute_query.js" we have used axios to execute the query and in case of e= xception, object is different then normal javascript response object.=C2=A0=
  • We call following functions when exception or error comes and send= the "<object>.response.data" as parameter=C2=A0
    • wasConnectionLostToServer= (): Check for the readyStat= e parameter, which is not the part of "<object>.response.= data".=C2=A0
    • extractE= rrorMessage(): Check for the "responseJSON" and "responseJSON.info"= ,=C2=A0which is not the part of "<object&g= t;.response.data".
    • is_pga_login_= required(): Check for the=C2=A0"responseJSON&= quot; and "responseJSON.info",=C2=A0which is not the= part of "<object>.response.data".
    • is_new_tr= ansaction_required():=C2=A0Check for the=C2= =A0"responseJSON" and "responseJSON.info",= =C2=A0which is not the part of "<ob= ject>.response.data".
From the above list, som= e 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 t= o identify the object is a axios<= /span> response or javascript response to above functions and change the lo= gic accordingly.

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 PM, Dave Page <<= a href=3D"mailto:dpage@pgadmin.org" target=3D"_blank">dpage@pgadmin.org= > wrote:
Thank= s, 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 th= e current pronlem


On Fri, Feb 9, 2018 at 7:29 AM Dave Page <dpage@pgadmin.org> wrote:
Hi Joao,

It loo= ks like Jenkins has taken umbrage to this change, at least with Python 3.x.= Can you take a look please?


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

On Fri, Feb 2, 2= 018 at 10:50 PM, Joao De Almeida Pereira <jdealmeidapereira@piv= otal.io> wrote:
Hi Hackers,
This is quite a big patch in order to solve the probl= em with the Explain Plan.

We sent 2 patches that h= ave 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-text-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 thei= r own files and add test around them
=C2=A0 Extract SQLEditor bac= kend 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 plan parameters as a JSON object in the start query call.<= /div>
=C2=A0 Extract the compile_template_name into a function that can= be used by the different places that try to select the version of the temp= late 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.enterprised= b.com
The Enterprise PostgreSQL Company



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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Co= mpany



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



--
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
--001a113c9c4096db510567d4ce37--