From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xi Shen Subject: Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment Date: Wed, 15 Jun 2016 04:01:06 +0000 Message-ID: References: <87r3c4em1r.fsf@saiph.selenimh> <87a8isdso8.fsf@saiph.selenimh> <87vb1c0yyn.fsf@saiph.selenimh> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=94eb2c0930d4568d380535492c31 Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:46356) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bD212-0005Ka-1f for Emacs-orgmode@gnu.org; Wed, 15 Jun 2016 00:01:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bD20z-00087M-BG for Emacs-orgmode@gnu.org; Wed, 15 Jun 2016 00:01:19 -0400 Received: from mail-oi0-x229.google.com ([2607:f8b0:4003:c06::229]:34610) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bD20z-00087C-4H for Emacs-orgmode@gnu.org; Wed, 15 Jun 2016 00:01:17 -0400 Received: by mail-oi0-x229.google.com with SMTP id d132so15750625oig.1 for ; Tue, 14 Jun 2016 21:01:16 -0700 (PDT) In-Reply-To: List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: "Emacs-orgmode@gnu.org" --94eb2c0930d4568d380535492c31 Content-Type: text/plain; charset=UTF-8 Hi Nicolas, I suppose I should put the news entry to ./etc/ORG-NEWS file, but into which version? I created below entry, please take look and let me know where do you want me to put it. *** Improved support to Microsoft SQL Server in =ob-sql.el= =ob-sql.el= library removes support to the ~msosql~ engine which uses the deprecated =osql= command line tool, and replaces it with ~mssql~ engine which uses the =sqlcmd= command line tool. Use with properties like this: #+BEGIN_EXAMPLE :engine mssql :dbhost :dbuser :dbpassword :database #+END_EXAMPLE If you want to use the *trusted connection* feature, omit *both* the =dbuser= and =dbpassword= properties and add =cmdline -E= to the properties. If your Emacs is running in a Cygwin environment, the =ob-sql.el= library can pass the converted path to the =sqlcmd= tool. I checked the code and it does not quote the arguments for me. It is a safe manner in Windows to always quote the path. So I will keep it. I have a question. Currently the table generated by mssql engine has the "affected rows" append to the end, like this. | memberid | username | xx | flags | |-------------------+----------+------+-------| | 1 | GPL | Indo | NULL | | 2 | GPL | Indo | NULL | | | | | | | (2 rows affected) | | | | I personally prefer to remove it. Do you or the org community has a preference about this? Maybe I should keep the behavior align with other engines? On Tue, Jun 14, 2016 at 9:02 PM Xi Shen wrote: > Hi Nicholas, > > Sure, I will add the news and declaration. > > The "format" is required to quote the filename, so the spaces won't > surprise sqlcmd. Maybe the arguments will be quoted when passing to the > command? > > On Tue, Jun 14, 2016, 19:52 Nicolas Goaziou > wrote: > >> Hello, >> >> Xi Shen writes: >> >> > I think I uploaded the wrong patch. Sorry~ Please check this one. >> >> It looks good. Thank you. I have one minor comment about it, see below. >> >> Also, could you provide an entry for ORG-NEWS since this is a user >> visible change? >> >> > +(defun org-babel-sql-convert-standard-filename (file) >> > + "Convert the file name to OS standard. >> > +In Cygwin environment, is uses Cygwin specific function to >> > +convert the file name and double quote it. Otherwise, uses Emacs >> > +standard conversion function." >> > + (if (fboundp 'cygwin-convert-file-name-to-windows) >> > + (format "\"%s\"" (cygwin-convert-file-name-to-windows file)) >> > + (convert-standard-filename file))) >> >> `cygwin-convert-file-name-to-windows' is going to generate a compilation >> warning. You need to declare it with `declare-function' at the top of >> the file. >> >> I'm also surprised that the function above doesn't return a string >> already. Are you sure the `format' part is needed? >> >> Regards, >> >> -- >> Nicolas Goaziou >> > -- > > > Thanks, > David S. > -- Thanks, David S. --94eb2c0930d4568d380535492c31 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Nicolas,

I suppose I should put the = news entry to ./etc/ORG-NEWS file, but into which version? I created below = entry, please take look and let me know where do you want me to put it.

*** Improved support to= Microsoft SQL Server in =3Dob-sql.el=3D
=3Dob-sql.el=3D library removes support to the ~msosql~ engine whic= h uses
the deprecated =3Dosql=3D = command line tool, and replaces it with ~mssql~
engine which uses the =3Dsqlcmd=3D command line tool.=C2=A0 = Use with properties
like this:

#+BEGIN_EXAMPLE
=C2= =A0:engine mssql
=C2=A0:dbhost &l= t;host.com>
=C2=A0:dbuser <username>
=C2=A0:dbpassword <secret>
=C2=A0:database <database>
#+END_EXAMPLE

=
If you want to use the *trusted = connection* feature,=C2=A0omit=C2=A0*both* the
=3Ddbuser=3D and =3Ddbpassword=3D properties and add =3Dcmdl= ine -E=3D to the
properties.

If your Emacs is running in a Cygwin environment, the =3Dob-sql.e= l=3D
library can pass the convert= ed path to the =3Dsqlcmd=3D tool.

I c= hecked the code and it does not quote the arguments for me. It is a safe ma= nner in Windows to always quote the path. So I will keep it.

=
I have a question. Currently the table generated by mssql engine= has the "affected rows" append to the end, like this.
=
=C2=A0 | =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0memberid | username | xx =C2=A0 | flags |
=C2=A0 |-------------------+----------+------+-------= |
=C2=A0 | =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 1 | GPL =C2=A0 =C2=A0 =C2=A0| Indo | NUL= L =C2=A0|
=C2=A0 | =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 2 | GPL =C2=A0 =C2=A0 =C2=A0| Ind= o | NULL =C2=A0|
=C2=A0 | =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0 |
=C2=A0 | (2 rows affected) | =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0 |

I personally prefer to remove it. Do you or t= he org community has a preference about this? Maybe I should keep the behav= ior align with other engines?


On Tue, Jun 14, 2016 at 9:02 PM Xi Shen <davidshen84@gmail.com> wrote:=
Hi Nicholas,

Sure, I will add the news and declaration.=C2=A0

<= p dir=3D"ltr">The "format" is required to quote the filename, so = the spaces won't surprise sqlcmd. Maybe the arguments will be quoted wh= en passing to the command?


On Tue, Jun 14, 2016= , 19:52 Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
Hello,

Xi Shen <davi= dshen84@gmail.com> writes:

> I think I uploaded the wrong patch. Sorry~ Please check this one.

It looks good. Thank you. I have one minor comment about it, see below.

Also, could you provide an entry for ORG-NEWS since this is a user
visible change?

> +(defun org-babel-sql-convert-standard-filename (file)
> +=C2=A0 "Convert the file name to OS standard.
> +In Cygwin environment, is uses Cygwin specific function to
> +convert the file name and double quote it. Otherwise, uses Emacs
> +standard conversion function."
> +=C2=A0 (if (fboundp 'cygwin-convert-file-name-to-windows)
> +=C2=A0 =C2=A0 =C2=A0 (format "\"%s\"" (cygwin-con= vert-file-name-to-windows file))
> +=C2=A0 =C2=A0 (convert-standard-filename file)))

`cygwin-convert-file-name-to-windows' is going to generate a compilatio= n
warning. You need to declare it with `declare-function' at the top of the file.

I'm also surprised that the function above doesn't return a string<= br> already. Are you sure the `format' part is needed?

Regards,

--
Nicolas Goaziou
--


Thanks,
David S.

--


Thanks,
David S.

--94eb2c0930d4568d380535492c31--