Re: PATCH: Graphincal explain integrated in sql editor

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.
- Facing issue to open QueryTool as there is some problem in require
module.
- Please add 'codemirror/addon/fold/foldgutter',
'codemirror/addon/fold/foldcode',
'codemirror/addon/fold/pgadmin-sqlfoldcode' files which has been removed
from your patch.
- Remove below code from sqleditor.js which is duplicated in _execute
function
-
- self.trigger(
- 'pgadmin-sqleditor:loading-icon:show',
- '{{ _('Initializing the query execution!') }}'
- )
- Clear the existing contents of the Explain tab when execute the query
without explain.
- If schema name is exists then please prefix the schema name to the
node.
- 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.

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