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

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