Re: PATCH: Format SQL (external tool) - Mailing list pgadmin-hackers

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

pgadmin-hackers by date:

Previous
From: Dave Page
Date:
Subject: Re: [PATCH] Add Commit/Rollback toolbar action
Next
From: Sanket Mehta
Date:
Subject: Re: [PATCH] Fix crash when disabling auto commit