Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1b1ANd-0006LL-7w for pgadmin-hackers@arkaria.postgresql.org; Fri, 13 May 2016 10:31:37 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1b1ANc-0002Ht-95 for pgadmin-hackers@arkaria.postgresql.org; Fri, 13 May 2016 10:31:36 +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.84_2) (envelope-from ) id 1b1ANb-0002Hk-Gr for pgadmin-hackers@postgresql.org; Fri, 13 May 2016 10:31:35 +0000 Received: from mail-vk0-x22a.google.com ([2607:f8b0:400c:c05::22a]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1b1ANX-0001Ag-08 for pgadmin-hackers@postgresql.org; Fri, 13 May 2016 10:31:34 +0000 Received: by mail-vk0-x22a.google.com with SMTP id m188so131741912vka.1 for ; Fri, 13 May 2016 03:31:30 -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:date:message-id:subject:from:to :cc; bh=nCsFEGruytzKmH/YlTMXJIJIUe6JJsO/m8LcLr3nqFs=; b=K9KETSwmPtvFhDuf2u7SKpsuypzAux57CiNMCzkTNiC1YxbU0Ukoyt6l82056SuJDz YRd7NhO8IZWmOCxlf9tO42pQOl+XxzNkeb11PWPlEGYP1hbdsvl0SqanH27YCtDJylWG ZKwaR10dOfVf3gTNl9a/sAELPlvKpxGXN2USaI/OIdAIx9cG7lNJdNg2P8xw55WUvg6c 11gpT/hfc5jD/vYQe3Xmt6YGgTC9CsOJdmolsjRefBqLXaS3NSgRx0ZqLAPXvC3uZ9fy wYD2nzWApDel5X62dbsdpTns9WNCpKLSSiQm1zmrTS5gGnxki/Lg8DYoCT6V5m0mR0Jn Usyg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=nCsFEGruytzKmH/YlTMXJIJIUe6JJsO/m8LcLr3nqFs=; b=BqKMp5uxKkOwUk3AT6DxxlSP3dqbBWV16fUbY9Lp5EZgnIFYvzi5JY7qPoxT+ZBGbZ /sbguMINPhCNJnndALLZb8ZWu3dMxs/EbXY2Ch/nItClGaciJW+QwnbGPoDA2ujXBJGh +htyQ3yW3436o7VObiGou9LoGis0nUERdq+w55roYaqCKx5PzKvVC3nZ/61COseNwURN MDK4cGlibgEge0d43bTNXP5oqIY+n4imF9U43sSkLfclXkuNIK9uzXuQRiKpY42sTu+D XOpnEoDMd6Yo3pHZ+sRV4pP5lArwYYxp8Bz4hKrVDfQ3kUdCtwzsYCkKp3BWsQ0hQT0k 9fQQ== X-Gm-Message-State: AOPr4FXY3SiqhqarQ9IvkjLea4RElguq+Cfp7aYvgTGE2YR4Ncg5sGG0RQe2kI8HfIb/4jT0wJp5eiBBFxWhD2sa MIME-Version: 1.0 X-Received: by 10.31.16.39 with SMTP id g39mr7190044vki.66.1463135489201; Fri, 13 May 2016 03:31:29 -0700 (PDT) Received: by 10.31.189.70 with HTTP; Fri, 13 May 2016 03:31:29 -0700 (PDT) In-Reply-To: References: Date: Fri, 13 May 2016 16:01:29 +0530 Message-ID: Subject: Re: PATCH: Graphincal explain integrated in sql editor From: Sanket Mehta To: Akshay Joshi Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary=001a1143221e17b1a80532b6c7fa X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --001a1143221e17b1a80532b6c7fa Content-Type: text/plain; charset=UTF-8 Hi, Apart from resolving all the issues mentioned in previous mail, All the explain options and auto rollback/auto commit are also added to preferences dialog. Any change to explains options or auto-rollback/auto-commit in sql editor will directly reflect its corresponding option in preference dialog. Regards, Sanket Mehta Sr Software engineer Enterprisedb On Fri, May 13, 2016 at 3:55 PM, Sanket Mehta wrote: > Hi, > > Revised patch is attached > Response in inline. > > > Regards, > Sanket Mehta > Sr Software engineer > Enterprisedb > > On Tue, May 10, 2016 at 2:54 PM, Akshay Joshi < > akshay.joshi@enterprisedb.com> wrote: > >> Hi Sanket >> >> On Tue, May 10, 2016 at 12:21 PM, Sanket Mehta > .com> wrote: >> >>> Hi, >>> >>> As previous patch was not applicable to latest pgadmin4 source code, >>> here is the new patch accommodating latest code. >>> Please do review it and send comments. >>> >> >> Following is my review comments >> >> - Remove Demo and sample code which you have used for testing before >> integration. >> >> Fixes > >> >> - Facing issue to open QueryTool as there is some problem in require >> module. >> >> Fixed > >> >> - Please add 'codemirror/addon/fold/foldgutter', 'codemirror/addon/fold/foldcode', >> 'codemirror/addon/fold/pgadmin-sqlfoldcode' files which has been >> removed from your patch. >> >> Fixed > >> >> - Remove below code from sqleditor.js which is duplicated in _execute >> function >> - >> - self.trigger( >> - 'pgadmin-sqleditor:loading-icon:show', >> - '{{ _('Initializing the query execution!') }}' >> - ) >> >> Fixed > >> >> - Clear the existing contents of the Explain tab when execute the >> query without explain. >> >> Fixed > >> >> - If schema name is exists then please prefix the schema name to the >> node. >> >> Fixed > >> >> - Please check the data is available before working on it in sqleditor >> .js at line no 1043 inside _render(). In case of "View Data" it >> throws an error. >> >> Fixed > >> >> >>> >>> Regards, >>> Sanket Mehta >>> Sr Software engineer >>> Enterprisedb >>> >>> On Mon, May 9, 2016 at 11:56 PM, Sanket Mehta < >>> sanket.mehta@enterprisedb.com> wrote: >>> >>>> Hi, >>>> >>>> Please ignore previous patch as there was an error in it. >>>> >>>> Error: >>>> Tooltip was not getting disappear when user moves cursor out of image. >>>> >>>> I have attached a proper patch with this mail. >>>> Please consider it for testing. >>>> >>>> Regards, >>>> Sanket Mehta >>>> Sr Software engineer >>>> Enterprisedb >>>> >>>> On Mon, May 9, 2016 at 8:49 PM, Sanket Mehta < >>>> sanket.mehta@enterprisedb.com> wrote: >>>> >>>>> Hi, >>>>> >>>>> PFA revised patch according to Ashesh's comments. >>>>> Please find my response inline. >>>>> >>>>> I am currently adding minimap feature in graphical explain. >>>>> I will send a new patch for the same. >>>>> >>>>> Regards, >>>>> Sanket Mehta >>>>> Sr Software engineer >>>>> Enterprisedb >>>>> >>>>> On Mon, Apr 25, 2016 at 4:36 PM, Ashesh Vashi < >>>>> ashesh.vashi@enterprisedb.com> wrote: >>>>> >>>>>> Hi Sanket, >>>>>> >>>>>> Please find the review comments. >>>>>> - Please add the missing 'explain.css'. >>>>>> >>>>> Done >>>>> >>>>>> - The application should be smart enough to handle conflict in >>>>>> options. >>>>>> i.e. >>>>>> Buffer is not a valid options without EXPLAIN ANALYZE. >>>>>> >>>>> Done >>>>> >>>>>> - A statement having EXPLAIN keywords with different format should at >>>>>> least render the output in the data-grid. >>>>>> i.e. EXPLAIN (FORMAT xml) SELECT * FROM xyz; >>>>>> >>>>> Done >>>>> >>>>>> - Please use the keywords used in the EXPLAIN statement in capital. >>>>>> >>>>> Done >>>>> >>>>>> - Explain should not work with empty string. >>>>>> >>>>> Done >>>>> >>>>>> - Font size in the tooltip is very small. >>>>>> >>>>> Done >>>>> >>>>>> >>>>>> >>>>> - Smoothing the zoom functionality. >>>>>> >>>>> Minimap will be added and zoom functionality will be removed. So it is >>>>> ignored. >>>>> >>>>> - Arrow marker is hardly visible. >>>>>> >>>>> Done. >>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> >>>>>> Thanks & Regards, >>>>>> >>>>>> Ashesh Vashi >>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company >>>>>> >>>>>> >>>>>> >>>>>> *http://www.linkedin.com/in/asheshvashi* >>>>>> >>>>>> >>>>>> On Mon, Apr 25, 2016 at 3:06 PM, Sanket Mehta < >>>>>> sanket.mehta@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> This patch includes the patch sent earlier for stand alone graphical >>>>>>> explain. >>>>>>> >>>>>>> And also "horizontal lines are not proper" bug is fixed in the same >>>>>>> which was reported by Dave in previous patch. >>>>>>> >>>>>>> Regards, >>>>>>> Sanket Mehta >>>>>>> Sr Software engineer >>>>>>> Enterprisedb >>>>>>> >>>>>>> On Thu, Apr 21, 2016 at 8:38 PM, Sanket Mehta < >>>>>>> sanket.mehta@enterprisedb.com> wrote: >>>>>>> >>>>>>>> Hi Team, >>>>>>>> >>>>>>>> PFA the first patch for graphical explain integrated in sql editor. >>>>>>>> >>>>>>>> Below are the few things which are different from previous patch >>>>>>>> which was sent for stand alone graphical explain. >>>>>>>> >>>>>>>> - Now user can select Explain/Explain Analyze with four optional >>>>>>>> properties (Verbose, costs, timing and buffers) >>>>>>>> >>>>>>>> - Initially graph will be scale (according to only its width not >>>>>>>> height) to fit to screen so no blank space will be there in case of very >>>>>>>> large graph. >>>>>>>> >>>>>>>> - Along with zoom in/out button, "zoom to original" button is also >>>>>>>> provided, by clicking on which graph will be scale to its original size >>>>>>>> (not same as initial one which is according to screen size). >>>>>>>> >>>>>>>> Please do review this patch and let me know in case you have any >>>>>>>> comments. >>>>>>>> >>>>>>>> >>>>>>>> Regards, >>>>>>>> Sanket Mehta >>>>>>>> Sr Software engineer >>>>>>>> Enterprisedb >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >>> >>> -- >>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) >>> To make changes to your subscription: >>> http://www.postgresql.org/mailpref/pgadmin-hackers >>> >>> >> >> >> -- >> *Akshay Joshi* >> *Principal Software Engineer * >> >> >> >> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246* >> > > --001a1143221e17b1a80532b6c7fa Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi,

Apart from resolving all the issues= mentioned in previous mail, All the explain options and auto rollback/auto= commit are also added to preferences dialog.
Any change to expla= ins options or auto-rollback/auto-commit in sql editor will directly reflec= t its corresponding option in preference dialog.


=
Regards,
Sanket Mehta
Sr Software engineer
Enterprisedb

On Fri, May 13, 2016 at 3:55 PM, Sanket Meht= a <sanket.mehta@enterprisedb.com> wrote:
Hi,

Revised = patch is attached
Response in inline.


Regards,
Sanket Mehta
Sr Software engineer
Enterprisedb

On Tue, May 10, 2016= at 2:54 PM, Akshay Joshi <akshay.joshi@enterprisedb.com&g= t; wrote:
Hi Sanket

On Tue, May 10, 2016 at 12:21 PM, Sanket Mehta <sanket.mehta@enterprisedb.com> wrote:
Hi,

As previous patch was not applicable to lates= t pgadmin4 source code, here is the new patch accommodating latest code.
Please do review it and send comments.

=C2=A0 =C2=A0 Following is my review comments=C2=A0<= /div>
  • Remove Demo and sample code which you have used for testi= ng before integration.
Fixes=C2=A0
  • Facing issue to open=C2=A0QueryTool=C2=A0as = there is some problem in require module.
  • <= /blockquote>
    Fixed=C2=A0
    • Please add=C2=A0'codemirror/addon/fold/foldgutter', 'codemirror/addon<= /span>/fold/foldcode', 'codemirror/addon/= fold/pgadmin-sqlfoldcode' files which has bee= n removed from your patch.
    Fixed=C2=A0
    =
    • Remove below code from=C2=A0sqleditor.js=C2=A0which is duplic= ated in _execute function
      • self.trigger(
      • =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0'pgadmin-sqleditor:loading-icon:show',
      • =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 '{{ _('Initializing the query execution!') }}'
      • = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 )
    <= /div>
    Fixed=C2=A0
    • Clear the existing contents of the Explain ta= b when execute the query without explain.
    =
    Fixed=C2=A0
    • If schema name is exists then please prefix the sch= ema name to the node.=C2=A0
    <= /span>
    Fixed=C2=A0
    • Please check the data is available before working on it in=C2=A0<= span>sqleditor.= js=C2=A0at line no 1043 inside _render().=C2=A0 In case of &q= uot;View Data" it throws an error.
    Fixed=C2=A0
    =C2=A0

    Regards= ,
    Sanket Mehta
    Sr Software engineer
    Enterprisedb

    On Mon, May 9, 2016 at 11:5= 6 PM, Sanket Mehta <sanket.mehta@enterprisedb.com> wrote:
    Hi,

    Pleas= e ignore previous patch as there was an error in it.

    Error:
    Tooltip was not getting disappear when user moves curso= r out of image.

    I have attached a proper patch wit= h this mail.
    Please consider it for testing.

    Regards,
    = Sanket Mehta
    Sr Software engineer
    Enterprisedb

    On Mon, May 9, 2016 at 8:49= PM, Sanket Mehta <sanket.mehta@enterprisedb.com> wrote:
    Hi,

    PFA re= vised patch according to Ashesh's comments.
    Please find my re= sponse inline.

    I am currently adding minimap featu= re in graphical explain.
    I will send a new patch for the same.

    Regards,
    Sanket Mehta
    Sr Software engineer
    En= terprisedb

    On Mon, Apr 25, 2016 at 4:36 PM= , Ashesh Vashi <ashesh.vashi@enterprisedb.com> w= rote:
    Hi Sanket,

    Pl= ease find the review comments.
    - Please add the missing 'expl= ain.css'.
    Done=C2=A0
    - The application should be smart enoug= h to handle conflict in options.
    =C2=A0 =C2=A0i.e.
    =C2= =A0 =C2=A0Buffer is not a valid options without EXPLAIN ANALYZE.
    Done=C2=A0
    - A statement having EXPLAIN keywords with different format s= hould at least render the output in the data-grid.
    =C2=A0 i.e. EX= PLAIN (FORMAT xml) SELECT * FROM xyz;
    D= one=C2=A0
    - Please use th= e keywords used in the EXPLAIN statement in capital.
    Done=C2=A0
    = - Explain should not work with empty string.
    Done=C2=A0
    - Font s= ize in the tooltip is very small.
    Done= =C2=A0
    =C2=A0
    =
    - Smoothing the zoom fu= nctionality.
    Minimap will be added and = zoom functionality will be removed. So it is ignored.

    <= /div>
    - Arrow marker is hardly visibl= e.
    Done.=C2=A0

    --<= /p>

    Thanks & Regards,

    Ashesh Vashi
    EnterpriseDB INDIA: Enterprise PostgreSQ= L Company


    http://www.linkedin.com/in/asheshvashi


    On Mon, Apr 25, 2016 at 3:06 PM, Sanket Meht= a <sanket.mehta@enterprisedb.com> wrote:
    Hi,

    This patch includes th= e patch sent earlier for stand alone graphical explain.

    And also "horizontal lines are not proper" bug is fixed in = the same which was reported by Dave in previous patch.

    Regard= s,
    Sanket Mehta
    Sr Software engineer
    Enterprisedb

    On Thu, Apr 21, 2016 at 8:3= 8 PM, Sanket Mehta <sanket.mehta@enterprisedb.com> wrote:
    Hi Team,

    = PFA the first patch for graphical explain integrated in sql editor.

    Below are the few things which are different from previou= s patch which was sent for stand alone graphical explain.

    =C2=A0- =C2=A0Now user can select Explain/Explain Analyze with four= optional properties (Verbose, costs, timing and buffers)

    =C2=A0- Initially graph will be scale (according to only its width = not height) to fit to screen so no blank space will be there in case of ver= y large graph.

    - Along with zoom in/out button, &q= uot;zoom to original" button is also provided, by clicking on which gr= aph will be scale to its original size (not same as initial one which is ac= cording to screen size).

    Please do review this pat= ch and let me know in case you have any comments.

    =
    Regards,
    Sanket Mehta
    Sr Software engineer
    Enterprisedb







    --
    Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
    To make changes to your subscription:
    http://www.postgresql.org/mailpref/pgadmin-hackers=




    --
    Akshay Joshi<= /span>
    Principal Software Engineer=C2=A0


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


    --001a1143221e17b1a80532b6c7fa--