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 <host.com>
 :dbuser <username>
 :dbpassword <secret>
 :database <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 <davidshen84@gmail.com> 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 <mail@nicolasgoaziou.fr>
> wrote:
>
>> Hello,
>>
>> Xi Shen <davidshen84@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)
>> > +  "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.