Re: PATCH: Graphincal explain integrated in sql editor

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*
>