Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment

[Xi Shen <davidshen84@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3387 bytes --]

Hi Nicolas,

Please see my updated patch.


On Sun, Jun 12, 2016 at 6:37 PM Xi Shen <davidshen84@gmail.com> wrote:

> Hi Nicolas,
>
> So I will:
>
> - add "org-babel-sql-convert-filename", so another name...I am thinking
> - remove `msosql` support. I am been playing with the options for a while,
> and I could not find a way the make osql output the same format as sqlcmd
>
>
> Thanks,
> David
>
>
> On Sun, Jun 12, 2016 at 10:12 AM Xi Shen <davidshen84@gmail.com> wrote:
>
>> Yes, I think it is better to let upstream function to resolve the path
>> for org-mode.
>>
>> But I have never contacted Emacs developers before. Should I go through
>> the bug-gnu-emacs@gnu.org mail list? Or there's a more effective channel?
>>
>>
>> On Sat, Jun 11, 2016 at 4:41 PM Nicolas Goaziou <mail@nicolasgoaziou.fr>
>> wrote:
>>
>>> Hello,
>>>
>>> Xi Shen <davidshen84@gmail.com> writes:
>>>
>>> > According to
>>> >
>>> https://www.gnu.org/software/emacs/manual/html_node/elisp/Standard-File-Names.html
>>> ,
>>> > the `convert-standard-filename` works for *nix and MS-DOS, but not
>>> Cygwin
>>> > environment. And I tested, it does not work. For the prefix, please
>>> advice
>>> > me a better one. Maybe we should path this function first? How can I
>>> > patch/update a Emacs native function?
>>>
>>> Since there is no module in Emacs, you need to prefix functions and
>>> variables according to the package, or, even better, the library they
>>> belong to.
>>>
>>> Hence, functions and variables in "ob-sql.el" are prefixed with
>>> "org-babel-sql-".
>>>
>>> Do you mind discussing it upstream on emacs-devel ML first? I don't
>>> think this kind of function belongs to Org. If upstream has no
>>> equivalent and doesn't want to add one, we might consider adding it to
>>> the library.
>>>
>>> WDYT?
>>>
>>> >> > The `osql` command line tool was last updated in 2004,
>>> >> > https://technet.microsoft.com/en-us/library/aa214012(v=sql.80).aspx
>>> ,
>>> >> > and could not output the query result in a way that morden
>>> >> > `org-table.el` expects.  The `sqlcmd` is the preferred command line
>>> >> > tool to connect the Microsoft SQL Server and it also has a Linux
>>> >> > version,
>>> >> > https://msdn.microsoft.com/en-us/library/hh568447(v=sql.110).aspx.
>>> >>
>>> >> Would it make sense to remove the msosql support then?
>>> >>
>>> > Yes, but I am also thinking about backward compatibility. Do you want
>>> > me to create a patch to remove `msosql` support?
>>>
>>> AFAIU, according to your comment, "osql" output is barely usable. If you
>>> think it is still usable and even used by some users, then I do not mind
>>> keeping it. I just wanted to be sure we're not keeping something that is
>>> not reasonable to keep.
>>>
>>> >> #'identity
>>> >>
>>> >>
>>> >>> OK, but what's the difference? Care to give me a short lesson?
>>> >>>Thanks!
>>>
>>> Not much difference, hence the "nitpick" tag.
>>>
>>> 'identity is a generic symbol, #'identity clearly indicates we (the
>>> user, the compiler) are interested in the symbol function cell.
>>>
>>> In this case, it is obvious, but it is not always the case in other
>>> parts of the code base, and more consistency in the right direction
>>> doesn't hurt.
>>>
>>>
>>> Regards,
>>>
>>> --
>>> Nicolas Goaziou
>>>
>>>
>>
>> Thanks,
>> David
>>
>> --
>>
>> Thanks,
>> David S.
>>
> --
>
> Thanks,
> David S.
>
-- 

Thanks,
David S.

[-- Attachment #1.2: Type: text/html, Size: 5748 bytes --]

[-- Attachment #2: 0001-ob-sql.el-Support-sqlcmd-in-Cygwin-environment.patch --]
[-- Type: application/octet-stream, Size: 3457 bytes --]

From e631854a0eb90b0b0e361dcb9e1f1efe034984f0 Mon Sep 17 00:00:00 2001
From: Xi Shen <davidshen84@gmail.com>
Date: Wed, 8 Jun 2016 13:49:54 +0800
Subject: [PATCH] ob-sql.el: Support sqlcmd in Cygwin environment

* lisp/ob-sql.el (org-babel-sql-dbstring-mssql): Format Microsoft
  `sqlcmd' command line args.
(org-babel-sql-convert-standard-filename): Convert a Posix path to
Windows long path in Cygwin environment, or do nothing.
(org-babel-execute:sql): Add `mssql' command support and remove support
for `msosql'.

The `osql' command line tool was last updated in 2004,
https://technet.microsoft.com/en-us/library/aa214012(v=sql.80).aspx, and
could not output the query result in a way that morden `org-table.el'
expects.  The `sqlcmd' is the preferred command line tool to connect the
Microsoft SQL Server and it also has a Linux version,
https://msdn.microsoft.com/en-us/library/hh568447(v=sql.110).aspx.

TINYCHANGE
---
 lisp/ob-sql.el | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el
index 6488afe..ee0650c 100644
--- a/lisp/ob-sql.el
+++ b/lisp/ob-sql.el
@@ -103,6 +103,27 @@ Pass nil to omit that arg."
   "Make Oracle command line args for database connection."
   (format "%s/%s@%s:%s/%s" user password host port database))
 
+(defun org-babel-sql-dbstring-mssql (host user password database)
+  "Make sqlcmd commmand line args for database connection.
+`sqlcmd' is the preferred command line tool to access Microsoft
+SQL Server on Windows and Linux platform."
+  (mapconcat #'identity
+	     (delq nil
+		   (list (when host (format "-S \"%s\"" host))
+			 (when user (format "-U \"%s\"" user))
+			 (when password (format "-P \"%s\"" password))
+			 (when database (format "-d \"%s\"" database))))
+	     " "))
+
+(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)))
+
 (defun org-babel-execute:sql (body params)
   "Execute a block of Sql code with Babel.
 This function is called by `org-babel-execute-src-block'."
@@ -131,8 +152,18 @@ This function is called by `org-babel-execute-src-block'."
 				      (org-babel-process-file-name out-file)))
                     (`msosql (format "osql %s -s \"\t\" -i %s -o %s"
 				     (or cmdline "")
-				     (org-babel-process-file-name in-file)
-				     (org-babel-process-file-name out-file)))
+				     (org-babel-sql-convert-standard-filename
+				      (org-babel-process-file-name in-file))
+				     (org-babel-sql-convert-standard-filename
+				      (org-babel-process-file-name out-file))))
+		    (`mssql (format "sqlcmd %s -s \"\t\" %s -i %s -o %s"
+				     (or cmdline "")
+				     (org-babel-sql-dbstring-mssql
+				      dbhost dbuser dbpassword database)
+				     (org-babel-sql-convert-standard-filename
+				      (org-babel-process-file-name in-file))
+				     (org-babel-sql-convert-standard-filename
+				      (org-babel-process-file-name out-file))))
                     (`mysql (format "mysql %s %s %s < %s > %s"
 				    (org-babel-sql-dbstring-mysql
 				     dbhost dbport dbuser dbpassword database)
-- 
2.8.3