Quick Links

Re: PATCH: Format SQL (external tool)

From:	"J(dot)F(dot) Oster" <jinfroster(at)mail(dot)ru>
To:	Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
Cc:	Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Dave Page <dpage(at)pgadmin(dot)org>
Subject:	Re: PATCH: Format SQL (external tool)
Date:	2015-06-26 18:48:52
Message-ID:	3810583589.20150626214852@mail.ru
Views:	Raw Message \| Whole Thread \| Download mbox \| Resend email
Thread:
Lists:	pgadmin-hackers

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html><head><title></title>
<META http-equiv=Content-Type content="text/html; charset=utf-8">
<meta http-equiv="Content-Style-Type" content="text/css">
<style type="text/css"></style>
</head>
<body>

Hello Ashesh,
 
Sorry for long absense.
 
Are you sure we need that additional option for specifying timeout?
How do you see it placed in the Options dialogue? For me it will look somewhat surplus and "overloading" the form.
It will be needed to change extremely rarely. The only true case I can imagine is using a wrapper script for online formatting service.
I suggest to provide a value in sysSettings, but keep it hidden. If needed, the user will be able to change it by hand. The one who manages to use such advanced functionality IS a hacker already :) and won't feel much inconvenience anyway.
How do you think?
 
Regarding the events internals - honestly, it's too complicated for me...
The very first version of my code in case of a timeout could lead to SEGFAULT somewhere inside wxWidget's event handling internals, with no pgAdmin's code in the stack. 
After I've changed by guess the AbortProcess() code to it's current look, it never broke since that. But I can't tell anymore deeper, sorry... :(
 
 
Wednesday, May 27, 2015, 12:56:25 PM, you wrote:
 
<div><table border=0 cellpadding=1 cellspacing=2>
<tr valign=top>
<td width=12 style="background-color: #0000ff;">
>
</td>
<td width=1038 style="background-color: #ffffff;">
On Wed, May 27, 2015 at 3:19 PM, Ashesh Vashi <<a class=rvts8 href="mailto:ashesh(dot)vashi(at)enterprisedb(dot)com">ashesh(dot)vashi(at)enterprisedb(dot)com</a>> wrote:
Hi J. F. Oster,
 
I think - we should give option to the user about wait timeout (which is hard-coded to 3 seconds).
It should be asked in the options dialog.
Apart from that - in the AbortProcess function, we're releasing the process object.
Does EVT_END_PROCESS event get fired even in case of killing the process across all supported platform?
(FYI - I've not tested the code yet.)
 
 
--
Thanks & Regards,
 
Ashesh Vashi
EnterpriseDB INDIA: <a class=rvts10 href="http://www.enterprisedb.com">Enterprise PostgreSQL Company</a>
 
<a class=rvts11 href="http://www.linkedin.com/in/asheshvashi">http://www.linkedin.com/in/asheshvashi</a>
 
 
On Wed, May 27, 2015 at 10:56 AM, Akshay Joshi <<a class=rvts8 href="mailto:akshay(dot)joshi(at)enterprisedb(dot)com">akshay(dot)joshi(at)enterprisedb(dot)com</a>> wrote:
 
 
 
On Tue, May 26, 2015 at 9:51 PM, J.F. Oster <<a class=rvts8 href="mailto:jinfroster(at)mail(dot)ru">jinfroster(at)mail(dot)ru</a>> wrote:
 
Hello Akshay,
 
Is there something else to fix?
 
   Nothing. Patch looks good to me. 
 
JFO> Hi Akshay,
 
JFO> Removed that.
 
 
JFO> Tuesday, May 19, 2015, 10:04:59 AM, you wrote:
 
AJ>> Hi J.F
 
AJ>> The version of fsql you have suggested works for me as well. I
AJ>> have reviewed your patch and it looks good to me. Please remove
AJ>> the commented code (wxString s; //, tmp  at line 873 in sysSettings.cpp.
 
 
AJ>> On Mon, May 18, 2015 at 10:31 PM, J.F. Oster <<a class=rvts8 href="mailto:jinfroster(at)mail(dot)ru">jinfroster(at)mail(dot)ru</a>> wrote:
AJ>> Hi Akshay,
 
AJ>> fsqlf.exe is the program to use; wx_fsqlf.exe is just a GUI wrapper.
 
AJ>> I've got the latest version (fsqlf.v0.03-292-gd0fd9bf.zip), and it really fails to run
AJ>> Please try the previous one, it works for me.
AJ>> <a class=rvts8 href="http://sourceforge.net/projects/fsqlf/files/fsqlf.v0.03/fsqlf.v0.03-141-g94f5a5f.zip.gz/download">http://sourceforge.net/projects/fsqlf/files/fsqlf.v0.03/fsqlf.v0.03-141-g94f5a5f.zip.gz/download</a>
 
AJ>> Also please note that fsqlf.exe could fail when run in a path containing national characters.
 
 
 
AJ>> Monday, May 18, 2015, 3:42:11 PM, you wrote:
 
 
 
 
AJ>> Hi J.F
 
AJ>> I am reviewing your patch. I have applied the patch and try to
AJ>> test it on Windows 7. Below are the steps that I perform
AJ>> °       Download SQL Formatter from <a class=rvts8 href="http://fsqlf.sourceforge.net/">http://fsqlf.sourceforge.net/</a>
AJ>> °       Given the path of fsqlf.exe/wx_fsqlf.exe in File -
AJ>> Options - Query Editor: External formatting utility
AJ>> °       I have opened the query tool and wrote some select query.
AJ>> Please refer the attached screenshot for SQL query.
AJ>> When I have given fsqlf.exe in the path it throws the error ( see
AJ>> attached screenshot) and when I have given wx_fsqlf.exe in the
AJ>> path it always report an error "Formatting command did not respond
AJ>> in 3 seconds" in the status bar.
 
AJ>> I am not sure how to test it properly. Can you please provide some steps.
 
 
AJ>> On Mon, May 18, 2015 at 10:10 AM, Akshay Joshi
AJ>> <<a class=rvts8 href="mailto:akshay(dot)joshi(at)enterprisedb(dot)com">akshay(dot)joshi(at)enterprisedb(dot)com</a>> wrote:
 
AJ>> Sure.
 
 
AJ>> On Fri, May 15, 2015 at 9:30 PM, Dave Page <<a class=rvts8 href="mailto:dpage(at)pgadmin(dot)org">dpage(at)pgadmin(dot)org</a>> wrote:
 
AJ>> Akshay, can you take a look please?
 
AJ>> Thanks.
 
 
AJ>> On Fri, May 15, 2015 at 4:53 PM, J.F. Oster <<a class=rvts8 href="mailto:jinfroster(at)mail(dot)ru">jinfroster(at)mail(dot)ru</a>> wrote:
>>> Hello!
 
>>> Please take a look at the patch.
>>> Thanks.
 
>>> Per discussion
>>> <a class=rvts8 href="http://www.postgresql.org/message-id/CAPyomk5NT9Tm-r3wombLzoY60Vqa+QyRDy4u84_2K9UWLbWHTg@mail.gmail.com">http://www.postgresql.org/message-id/CAPyomk5NT9Tm-r3wombLzoY60Vqa+QyRDy4u84_2K9UWLbWHTg@mail.gmail.com</a>
 
>>> It's most useful for making readable queries generated by ORMs such as
>>> Hibernate. But in general, external processing can go far beyond
>>> formatting task.
 
>>> I've implemented this feature quick-and-dirty long ago. Finally I made
>>> myself clean it up, now it looks better, so please consider a patch.
>>> Tested on Windows 7 and Ubuntu 14.04.
 
>>> Changes:
>>> * added new setting, ExtFormatCmd, "External formatting utility" in
>>>   Options dialogue
>>> * added menu item "Edit - Format - External Format" in
>>>   Query editor
>>> * class sysProcess supports UTF-8 and can pass STDIN for a process.
 
>>> Suggested use scenario:
>>> 1. Download and install some SQL formatting utility.
>>> 2. Tell pgAdmin where it resides:
>>>    File - Options - Query Editor: External formatting utility.
>>> 3. Open Query editor. Select a text block to format and press
>>>    Ctrl-Shift-F. With no selection the whole text gets formatted.
>>>    In case of non-zero exit code, STDERR will be shown in status bar.
 
>>> Requirements for external formatting utility:
>>> * Accepts a STDIN stream and writes result to STDOUT
>>> * Finishes in less than 3 seconds
>>> * Exits with code 0 on success
>>> Support for UTF-8 multibyte characters is preferable.
 
>>> To see whether it works well, a test can be done:
>>> C:\> type in.sql |some_formatter >out.sql
>>> C:\> echo %ERRORLEVEL%
>>> or
>>> user(at)linux:~$ cat in.sql |some_formatter >out.sql
>>> user(at)linux:~$ echo $?
 
>>> There are few available utilities depending on platform:
>>>    * Free SQL Formatter (Linux, Windows, Mac OS X(?))
>>>      <a class=rvts8 href="http://fsqlf.sourceforge.net/">http://fsqlf.sourceforge.net/</a>
>>>    * Poor Man's T-SQL Formatter (Windows)
>>>      <a class=rvts8 href="http://architectshack.com/PoorMansTSqlFormatter.ashx">http://architectshack.com/PoorMansTSqlFormatter.ashx</a>
>>> Also it is possible to make a wrapper script for numerous online
>>> formatting services, but it's less secure and less reliable.
 
>>> Fsqlf is FOSS and seems promising. I think of extending it for
>>> PosgreSQL-specific SQL syntax and probably even PL/pgSQL.
 
 
 
 
 
--
Best regards,
 J.F.
 
 
 
 
 
-- 
 
Akshay Joshi
Principal Software Engineer 
 
 
Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
</td>
</tr>
</table>
</div>
 
 
 
 
-- 
Best regards,
 J.F.

</body></html>

Attachment	Content-Type	Size
unknown_filename	text/html	14.5 KB

In response to

Re: Fwd: PATCH: Format SQL (external tool) at 2015-05-27 09:56:25 from Ashesh Vashi

Responses

Re: PATCH: Format SQL (external tool) at 2015-07-18 12:06:46 from J.F. Oster

Browse pgadmin-hackers by date

	From	Date	Subject
Next Message	Sanket Mehta	2015-06-29 11:05:47	Re: [PATCH] Fix crash when disabling auto commit
Previous Message	Dave Page	2015-06-25 07:58:00	Re: [PATCH] Add Commit/Rollback toolbar action