Re: PATCH: Graphincal explain integrated in sql editor - Mailing list pgadmin-hackers

From Ashesh Vashi
Subject Re: PATCH: Graphincal explain integrated in sql editor
Date
Msg-id CAG7mmoxrhu9uvbB4-oQuCaYO3BcDW-KnXptQb5P0tDj2b3ExBg@mail.gmail.com
Whole thread Raw
In response to Re: PATCH: Graphincal explain integrated in sql editor  (Sanket Mehta <sanket.mehta@enterprisedb.com>)
List pgadmin-hackers

On Fri, May 13, 2016 at 4:01 PM, Sanket Mehta <sanket.mehta@enterprisedb.com> wrote:

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.
Thanks - committed.

Looking forward to a minimap kind of functionality for better zooming, and sliding functionalities integrated with the explain module.
Also - added nice to have TODO list for the 'Graphical Explain' module.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company




Regards,
Sanket Mehta
Sr Software engineer
Enterprisedb

On Fri, May 13, 2016 at 3:55 PM, Sanket Mehta <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> 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 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-9517
Mobile: +91 976-788-8246



pgadmin-hackers by date:

Previous
From: Ashesh Vashi
Date:
Subject: pgAdmin 4 commit: Integrate the graphical explain module in the Query E
Next
From: Ashesh Vashi
Date:
Subject: pgAdmin 4 commit: Add support for foreign server