* [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
@ 2016-06-08 10:24 Xi Shen
2016-06-10 22:06 ` Nicolas Goaziou
0 siblings, 1 reply; 17+ messages in thread
From: Xi Shen @ 2016-06-08 10:24 UTC (permalink / raw)
To: Emacs-orgmode@gnu.org
[-- Attachment #1.1: Type: text/plain, Size: 327 bytes --]
Hi,
I would like to apply this path to add sqlcmd support, and allow org-mode
to execute and capture sqlcmd output in cygwin environment.
I added a "platform-convert-file-name" function to convert a *nix path to
Windows path. Should I put this function in ob-sql.el, or somewhere else?
Thanks,
David
--
Thanks,
David S.
[-- Attachment #1.2: Type: text/html, Size: 580 bytes --]
[-- Attachment #2: 0001-ob-sql.el-Support-sqlcmd-and-cygwin-environment.patch --]
[-- Type: application/octet-stream, Size: 3204 bytes --]
From 96daeb8d22e33e3484fefb27914bd434efd32142 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 and cygwin environment
* lisp/ob-sql.el (org-babel-sql-dbstring-mssql): Format Microsoft
sqlcmd command line args.
* lisp/ob-sql.el (platform-convert-file-name): Convert a *nix path to
Windows long path in Cygwin environment, or do nothing.
* lisp/ob-sql.el (org-babel-execute:sql): Add `mssql` command support.
For both `msosql` and `mssql` the `input` and `output` path will be
mapped by `platform-convert-file-name`
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 | 28 ++++++++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el
index 6488afe..b115faa 100644
--- a/lisp/ob-sql.el
+++ b/lisp/ob-sql.el
@@ -103,6 +103,24 @@ 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 Microsoft sqlcmd commmand line args for database
+connection."
+ (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 platform-convert-file-name (file)
+ "In Cygwin environment convert the file path into Windows long
+path and quote it."
+ (if (fboundp 'cygwin-convert-file-name-to-windows)
+ (format "\"%s\"" (cygwin-convert-file-name-to-windows file))
+ 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 +149,14 @@ 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)))
+ (platform-convert-file-name (org-babel-process-file-name in-file))
+ (platform-convert-file-name (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)
+ (platform-convert-file-name (org-babel-process-file-name in-file))
+ (platform-convert-file-name (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.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-08 10:24 [PATCH] ob-sql.el: Support sqlcmd and cygwin environment Xi Shen
@ 2016-06-10 22:06 ` Nicolas Goaziou
2016-06-11 2:17 ` Xi Shen
0 siblings, 1 reply; 17+ messages in thread
From: Nicolas Goaziou @ 2016-06-10 22:06 UTC (permalink / raw)
To: Xi Shen; +Cc: Emacs-orgmode@gnu.org
Hello,
Xi Shen <davidshen84@gmail.com> writes:
> I would like to apply this path to add sqlcmd support, and allow org-mode
> to execute and capture sqlcmd output in cygwin environment.
Thank you for the patch.
> I added a "platform-convert-file-name" function to convert a *nix path to
> Windows path. Should I put this function in ob-sql.el, or somewhere
> else?
I'm surprised it doesn't exist. Wouldn't `convert-standard-filename' do
the job?
In any case, it doesn't have a proper prefix.
> Subject: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
>
> * lisp/ob-sql.el (org-babel-sql-dbstring-mssql): Format Microsoft
> sqlcmd command line args.
>
> * lisp/ob-sql.el (platform-convert-file-name): Convert a *nix path to
> Windows long path in Cygwin environment, or do nothing.
>
> * lisp/ob-sql.el (org-babel-execute:sql): Add `mssql` command support.
> For both `msosql` and `mssql` the `input` and `output` path will be
> mapped by `platform-convert-file-name`
You can write it like
* lisp/ob-sql.el (org-babel-sql-dbstring-mssql): Format Microsoft
sqlcmd command line args.
(platform-convert-file-name): Convert a *nix path to Windows long path
in Cygwin environment, or do nothing.
(org-babel-execute:sql): Add `mssql' command support. For both `msosql'
and `mssql` the `input` and `output' path will be mapped by
`platform-convert-file-name'
> 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?
> +(defun org-babel-sql-dbstring-mssql (host user password database)
> + "Make Microsoft sqlcmd commmand line args for database
> +connection."
The first sentence has to fit on a single line.
> + (mapconcat 'identity
Nit-pick:
#'identity
> + "In Cygwin environment convert the file path into Windows long
> +path and quote it."
> + (if (fboundp 'cygwin-convert-file-name-to-windows)
> + (format "\"%s\"" (cygwin-convert-file-name-to-windows file))
> + file))
See above.
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-10 22:06 ` Nicolas Goaziou
@ 2016-06-11 2:17 ` Xi Shen
2016-06-11 8:40 ` Nicolas Goaziou
0 siblings, 1 reply; 17+ messages in thread
From: Xi Shen @ 2016-06-11 2:17 UTC (permalink / raw)
To: Xi Shen, Emacs-orgmode@gnu.org
[-- Attachment #1: Type: text/plain, Size: 3299 bytes --]
Hello Nicolas,
Please see my replies inline.
On Sat, Jun 11, 2016 at 6:06 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:
> Hello,
>
> Xi Shen <davidshen84@gmail.com> writes:
>
> > I would like to apply this path to add sqlcmd support, and allow org-mode
> > to execute and capture sqlcmd output in cygwin environment.
>
> Thank you for the patch.
>
> > I added a "platform-convert-file-name" function to convert a *nix path to
> > Windows path. Should I put this function in ob-sql.el, or somewhere
> > else?
>
> I'm surprised it doesn't exist. Wouldn't `convert-standard-filename' do
> the job?
>
> In any case, it doesn't have a proper prefix.
>
>
>> 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?
> > Subject: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
> >
> > * lisp/ob-sql.el (org-babel-sql-dbstring-mssql): Format Microsoft
> > sqlcmd command line args.
> >
> > * lisp/ob-sql.el (platform-convert-file-name): Convert a *nix path to
> > Windows long path in Cygwin environment, or do nothing.
> >
> > * lisp/ob-sql.el (org-babel-execute:sql): Add `mssql` command support.
> > For both `msosql` and `mssql` the `input` and `output` path will be
> > mapped by `platform-convert-file-name`
>
> You can write it like
>
> * lisp/ob-sql.el (org-babel-sql-dbstring-mssql): Format Microsoft
> sqlcmd command line args.
> (platform-convert-file-name): Convert a *nix path to Windows long path
> in Cygwin environment, or do nothing.
> (org-babel-execute:sql): Add `mssql' command support. For both `msosql'
> and `mssql` the `input` and `output' path will be mapped by
> `platform-convert-file-name'
>
> >> Sure, I will update the ChangeLog entry.
> > 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?
> > +(defun org-babel-sql-dbstring-mssql (host user password database)
> > + "Make Microsoft sqlcmd commmand line args for database
> > +connection."
>
> The first sentence has to fit on a single line.
>
> > Will do, thanks~
> > + (mapconcat 'identity
>
> Nit-pick:
>
> #'identity
>
>
>> OK, but what's the difference? Care to give me a short lesson? Thanks!
> > + "In Cygwin environment convert the file path into Windows long
> > +path and quote it."
> > + (if (fboundp 'cygwin-convert-file-name-to-windows)
> > + (format "\"%s\"" (cygwin-convert-file-name-to-windows file))
> > + file))
>
> See above.
>
> Regards,
>
> --
> Nicolas Goaziou
>
Thanks,
David
--
Thanks,
David S.
[-- Attachment #2: Type: text/html, Size: 5358 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-11 2:17 ` Xi Shen
@ 2016-06-11 8:40 ` Nicolas Goaziou
2016-06-12 2:12 ` Xi Shen
0 siblings, 1 reply; 17+ messages in thread
From: Nicolas Goaziou @ 2016-06-11 8:40 UTC (permalink / raw)
Cc: Emacs-orgmode@gnu.org
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-11 8:40 ` Nicolas Goaziou
@ 2016-06-12 2:12 ` Xi Shen
2016-06-12 10:37 ` Xi Shen
0 siblings, 1 reply; 17+ messages in thread
From: Xi Shen @ 2016-06-12 2:12 UTC (permalink / raw)
To: Emacs-orgmode@gnu.org
[-- Attachment #1: Type: text/plain, Size: 2665 bytes --]
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.
[-- Attachment #2: Type: text/html, Size: 4248 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-12 2:12 ` Xi Shen
@ 2016-06-12 10:37 ` Xi Shen
2016-06-13 5:36 ` Xi Shen
0 siblings, 1 reply; 17+ messages in thread
From: Xi Shen @ 2016-06-12 10:37 UTC (permalink / raw)
To: Emacs-orgmode@gnu.org
[-- Attachment #1: Type: text/plain, Size: 3125 bytes --]
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.
[-- Attachment #2: Type: text/html, Size: 5193 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-12 10:37 ` Xi Shen
@ 2016-06-13 5:36 ` Xi Shen
2016-06-13 6:30 ` Xi Shen
0 siblings, 1 reply; 17+ messages in thread
From: Xi Shen @ 2016-06-13 5:36 UTC (permalink / raw)
To: Emacs-orgmode@gnu.org
[-- 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-13 5:36 ` Xi Shen
@ 2016-06-13 6:30 ` Xi Shen
2016-06-14 11:52 ` Nicolas Goaziou
0 siblings, 1 reply; 17+ messages in thread
From: Xi Shen @ 2016-06-13 6:30 UTC (permalink / raw)
To: Emacs-orgmode@gnu.org
[-- Attachment #1.1: Type: text/plain, Size: 3695 bytes --]
Hi Nicolas,
I think I uploaded the wrong patch. Sorry~ Please check this one.
On Mon, Jun 13, 2016 at 1:36 PM Xi Shen <davidshen84@gmail.com> wrote:
> 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.
>
--
Thanks,
David S.
[-- Attachment #1.2: Type: text/html, Size: 6504 bytes --]
[-- Attachment #2: 0001-ob-sql.el-Support-sqlcmd-in-Cygwin-environment.patch --]
[-- Type: application/octet-stream, Size: 3299 bytes --]
From 2a2d1792f22d2917bb9ff61e7eb756085f38da2b 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 | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el
index 6488afe..74cd39f 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'."
@@ -129,10 +150,14 @@ This function is called by `org-babel-execute-src-block'."
(or cmdline "")
(org-babel-process-file-name in-file)
(org-babel-process-file-name out-file)))
- (`msosql (format "osql %s -s \"\t\" -i %s -o %s"
+ (`mssql (format "sqlcmd %s -s \"\t\" %s -i %s -o %s"
(or cmdline "")
- (org-babel-process-file-name in-file)
- (org-babel-process-file-name out-file)))
+ (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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-13 6:30 ` Xi Shen
@ 2016-06-14 11:52 ` Nicolas Goaziou
2016-06-14 13:02 ` Xi Shen
0 siblings, 1 reply; 17+ messages in thread
From: Nicolas Goaziou @ 2016-06-14 11:52 UTC (permalink / raw)
To: Xi Shen; +Cc: Emacs-orgmode@gnu.org
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-14 11:52 ` Nicolas Goaziou
@ 2016-06-14 13:02 ` Xi Shen
2016-06-15 4:01 ` Xi Shen
0 siblings, 1 reply; 17+ messages in thread
From: Xi Shen @ 2016-06-14 13:02 UTC (permalink / raw)
To: Emacs-orgmode@gnu.org
[-- Attachment #1: Type: text/plain, Size: 1374 bytes --]
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.
[-- Attachment #2: Type: text/html, Size: 2058 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-14 13:02 ` Xi Shen
@ 2016-06-15 4:01 ` Xi Shen
2016-06-15 16:49 ` Nicolas Goaziou
0 siblings, 1 reply; 17+ messages in thread
From: Xi Shen @ 2016-06-15 4:01 UTC (permalink / raw)
To: Emacs-orgmode@gnu.org
[-- Attachment #1: Type: text/plain, Size: 3114 bytes --]
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.
[-- Attachment #2: Type: text/html, Size: 5426 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-15 4:01 ` Xi Shen
@ 2016-06-15 16:49 ` Nicolas Goaziou
2016-06-16 6:04 ` Xi Shen
0 siblings, 1 reply; 17+ messages in thread
From: Nicolas Goaziou @ 2016-06-15 16:49 UTC (permalink / raw)
To: Xi Shen; +Cc: Emacs-orgmode@gnu.org
Hello,
Xi Shen <davidshen84@gmail.com> writes:
> 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.
I'd say
Version 9.0 > New features > Babel
or
Version 9.0 > Miscellaneous
> *** 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.
It looks good.
> 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.
Fair enough.
> 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?
I lean towards removing it, too. I doesn't give useful feedback. We can
always insert it back later if it introduces unwanted side-effects.
Thank you.
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-15 16:49 ` Nicolas Goaziou
@ 2016-06-16 6:04 ` Xi Shen
2016-06-16 8:56 ` tumashu
2016-06-16 22:29 ` Nicolas Goaziou
0 siblings, 2 replies; 17+ messages in thread
From: Xi Shen @ 2016-06-16 6:04 UTC (permalink / raw)
To: Emacs-orgmode@gnu.org
[-- Attachment #1.1: Type: text/plain, Size: 2412 bytes --]
Hi Nicolas,
Please take a look at the updated patch. Changes:
- add ORG-NEWS entry
- add function declaration
- add input file template for `mssql' engine to remove the "affected rows"
tail
On Thu, Jun 16, 2016 at 12:49 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:
> Hello,
>
> Xi Shen <davidshen84@gmail.com> writes:
>
> > 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.
>
> I'd say
>
> Version 9.0 > New features > Babel
>
> or
>
> Version 9.0 > Miscellaneous
>
> > *** 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.
>
> It looks good.
>
> > 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.
>
> Fair enough.
>
> > 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?
>
> I lean towards removing it, too. I doesn't give useful feedback. We can
> always insert it back later if it introduces unwanted side-effects.
>
>
> Thank you.
>
>
> Regards,
>
> --
> Nicolas Goaziou
>
--
Thanks,
David S.
[-- Attachment #1.2: Type: text/html, Size: 3447 bytes --]
[-- Attachment #2: 0001-ob-sql.el-Support-sqlcmd-in-Cygwin-environment.patch --]
[-- Type: application/octet-stream, Size: 5066 bytes --]
From e689bb17dc7f644f71e884d0b06013359d2f7a70 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
* etc/ORG-NEWS: Add "Improved support to Microsoft SQL Server via
~sqlcmd~" section to addresss this change.
* 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' engine support and remove support
for `msosql' engine.
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
---
etc/ORG-NEWS | 20 ++++++++++++++++++++
lisp/ob-sql.el | 36 +++++++++++++++++++++++++++++++++---
2 files changed, 53 insertions(+), 3 deletions(-)
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index ed44a21..8af7423 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -210,6 +210,26 @@ mandatory):
:database <database>
:dbpassword <secret>
#+END_EXAMPLE
+**** Improved support to Microsoft SQL Server via ~sqlcmd~
+=ob-sql= 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= library
+can pass the converted path to the =sqlcmd= tool.
**** Support for additional plantuml output formats
The support for output formats of [[http://plantuml.com/][plantuml]] has been extended to now
include:
diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el
index 6488afe..fdf73a0 100644
--- a/lisp/ob-sql.el
+++ b/lisp/ob-sql.el
@@ -62,6 +62,7 @@
(declare-function org-table-import "org-table" (file arg))
(declare-function orgtbl-to-csv "org-table" (table params))
(declare-function org-table-to-lisp "org-table" (&optional txt))
+(declare-function cygwin-convert-file-name-to-windows "cygw32.c" (file &optional absolute-p))
(defvar org-babel-default-header-args:sql '())
@@ -103,6 +104,28 @@ 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.
+If in Cygwin environment, uses Cygwin specific function to
+convert the file name. Otherwise, uses Emacs' standard conversion
+function."
+ (format "\"%s\""
+ (if (fboundp 'cygwin-convert-file-name-to-windows)
+ (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'."
@@ -129,10 +152,14 @@ This function is called by `org-babel-execute-src-block'."
(or cmdline "")
(org-babel-process-file-name in-file)
(org-babel-process-file-name out-file)))
- (`msosql (format "osql %s -s \"\t\" -i %s -o %s"
+ (`mssql (format "sqlcmd %s -s \"\t\" %s -i %s -o %s"
(or cmdline "")
- (org-babel-process-file-name in-file)
- (org-babel-process-file-name out-file)))
+ (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)
@@ -173,6 +200,9 @@ SET MARKUP HTML OFF SPOOL OFF
SET COLSEP '|'
")
+ (`mssql "SET NOCOUNT ON
+
+")
(_ ""))
(org-babel-expand-body:sql body params)))
(org-babel-eval command "")
--
2.8.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-16 6:04 ` Xi Shen
@ 2016-06-16 8:56 ` tumashu
2016-06-16 22:29 ` Nicolas Goaziou
1 sibling, 0 replies; 17+ messages in thread
From: tumashu @ 2016-06-16 8:56 UTC (permalink / raw)
To: Xi Shen; +Cc: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 2460 bytes --]
You can use sqsh to connect MS-sql too, more info you can see: https://github.com/tumashu/sql-mssql
At 2016-06-16 14:04:26, "Xi Shen" <davidshen84@gmail.com> wrote:
Hi Nicolas,
Please take a look at the updated patch. Changes:
- add ORG-NEWS entry
- add function declaration
- add input file template for `mssql' engine to remove the "affected rows" tail
On Thu, Jun 16, 2016 at 12:49 AM Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
Hello,
Xi Shen <davidshen84@gmail.com> writes:
> 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.
I'd say
Version 9.0 > New features > Babel
or
Version 9.0 > Miscellaneous
> *** 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.
It looks good.
> 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.
Fair enough.
> 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?
I lean towards removing it, too. I doesn't give useful feedback. We can
always insert it back later if it introduces unwanted side-effects.
Thank you.
Regards,
--
Nicolas Goaziou
--
Thanks,
David S.
[-- Attachment #2: Type: text/html, Size: 4168 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-16 6:04 ` Xi Shen
2016-06-16 8:56 ` tumashu
@ 2016-06-16 22:29 ` Nicolas Goaziou
2016-06-20 12:34 ` Xi Shen
1 sibling, 1 reply; 17+ messages in thread
From: Nicolas Goaziou @ 2016-06-16 22:29 UTC (permalink / raw)
To: Xi Shen; +Cc: Emacs-orgmode@gnu.org
Hello,
Xi Shen <davidshen84@gmail.com> writes:
> Please take a look at the updated patch. Changes:
>
> - add ORG-NEWS entry
> - add function declaration
> - add input file template for `mssql' engine to remove the "affected rows"
> tail
Applied. Thank you.
Please consider signing FSF papers if you want to contribute more to
Org.
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-16 22:29 ` Nicolas Goaziou
@ 2016-06-20 12:34 ` Xi Shen
2016-07-04 8:11 ` Xi Shen
0 siblings, 1 reply; 17+ messages in thread
From: Xi Shen @ 2016-06-20 12:34 UTC (permalink / raw)
To: Emacs-orgmode@gnu.org
[-- Attachment #1: Type: text/plain, Size: 570 bytes --]
Thanks Nicolas! I will consider to sign FSF the next time :)
On Fri, Jun 17, 2016 at 6:29 AM Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:
> Hello,
>
> Xi Shen <davidshen84@gmail.com> writes:
>
> > Please take a look at the updated patch. Changes:
> >
> > - add ORG-NEWS entry
> > - add function declaration
> > - add input file template for `mssql' engine to remove the "affected
> rows"
> > tail
>
> Applied. Thank you.
>
> Please consider signing FSF papers if you want to contribute more to
> Org.
>
> Regards,
>
> --
> Nicolas Goaziou
>
--
Thanks,
David S.
[-- Attachment #2: Type: text/html, Size: 1082 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] ob-sql.el: Support sqlcmd and cygwin environment
2016-06-20 12:34 ` Xi Shen
@ 2016-07-04 8:11 ` Xi Shen
0 siblings, 0 replies; 17+ messages in thread
From: Xi Shen @ 2016-07-04 8:11 UTC (permalink / raw)
To: Emacs-orgmode@gnu.org
Hello Nicolas,
I saw my patch in the master for a while, but I still cannot get the
change from `org-plus-contrib' package...I have to manually patch it
on my local which is not very convenient. When could I expect my patch
release in the `org-plus-contrib' package?
Thanks,
David
On Mon, Jun 20, 2016 at 8:34 PM Xi Shen <davidshen84@gmail.com> wrote:
>
> Thanks Nicolas! I will consider to sign FSF the next time :)
>
>
> On Fri, Jun 17, 2016 at 6:29 AM Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
>>
>> Hello,
>>
>> Xi Shen <davidshen84@gmail.com> writes:
>>
>> > Please take a look at the updated patch. Changes:
>> >
>> > - add ORG-NEWS entry
>> > - add function declaration
>> > - add input file template for `mssql' engine to remove the "affected rows"
>> > tail
>>
>> Applied. Thank you.
>>
>> Please consider signing FSF papers if you want to contribute more to
>> Org.
>>
>> Regards,
>>
>> --
>> Nicolas Goaziou
>
> --
>
>
> Thanks,
> David S.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-07-04 8:12 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-08 10:24 [PATCH] ob-sql.el: Support sqlcmd and cygwin environment Xi Shen
2016-06-10 22:06 ` Nicolas Goaziou
2016-06-11 2:17 ` Xi Shen
2016-06-11 8:40 ` Nicolas Goaziou
2016-06-12 2:12 ` Xi Shen
2016-06-12 10:37 ` Xi Shen
2016-06-13 5:36 ` Xi Shen
2016-06-13 6:30 ` Xi Shen
2016-06-14 11:52 ` Nicolas Goaziou
2016-06-14 13:02 ` Xi Shen
2016-06-15 4:01 ` Xi Shen
2016-06-15 16:49 ` Nicolas Goaziou
2016-06-16 6:04 ` Xi Shen
2016-06-16 8:56 ` tumashu
2016-06-16 22:29 ` Nicolas Goaziou
2016-06-20 12:34 ` Xi Shen
2016-07-04 8:11 ` Xi Shen
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).