Re: PATCH: Graphincal explain integrated in sql editor

From: Sanket Mehta <sanket(dot)mehta(at)enterprisedb(dot)com>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: Graphincal explain integrated in sql editor
Date: 2016-05-13 10:31:29
Message-ID: CA+yw=mPQBVnJO+p-fGJXPWGepcmNTWjFHrSKNf2U8uLCr9=qAQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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 <sanket(dot)mehta(at)enterprisedb(dot)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(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi Sanket
>>
>> On Tue, May 10, 2016 at 12:21 PM, Sanket Mehta <sanket(dot)mehta(at)enterprisedb
>> .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(dot)mehta(at)enterprisedb(dot)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(dot)mehta(at)enterprisedb(dot)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(dot)vashi(at)enterprisedb(dot)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.enterprisedb.com>
>>>>>>
>>>>>>
>>>>>> *http://www.linkedin.com/in/asheshvashi*
>>>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>>>
>>>>>> On Mon, Apr 25, 2016 at 3:06 PM, Sanket Mehta <
>>>>>> sanket(dot)mehta(at)enterprisedb(dot)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(dot)mehta(at)enterprisedb(dot)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(at)postgresql(dot)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*
>>
>
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Surinder Kumar 2016-05-13 11:03:55 Re: [pgAdmin4][Patch]: File Manager & Backform FileControl
Previous Message Sanket Mehta 2016-05-13 10:25:47 Re: PATCH: Graphincal explain integrated in sql editor