[PATCH] ob-sql: Respect database param when using dbconnection

[PATCH] ob-sql: Respect database param when using dbconnection

[Daniel Kraus]

[-- Attachment #1: Type: text/plain, Size: 288 bytes --]

Hi,

I use ob-sql with the :dbconnection param so I don't have my username and password in my org file.
But often I don't want to use the default database from the dbconnection alist but
rather specify it explicitly with :database.
Attached is a patch that fixes this.

Thanks,
  Daniel


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] ob-sql: Respect database param when using dbconnection --]
[-- Type: text/x-patch, Size: 969 bytes --]

From a8dccff104d7426e2f353b1005e0bdcc51de6e99 Mon Sep 17 00:00:00 2001
From: Daniel Kraus <daniel@kraus.my>
Date: Tue, 26 May 2020 16:07:34 +0200
Subject: [PATCH] ob-sql: Respect database param when using dbconnection

---
 lisp/ob-sql.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el
index 7c359b988..d7a8bf0a0 100644
--- a/lisp/ob-sql.el
+++ b/lisp/ob-sql.el
@@ -191,7 +191,8 @@ then look for the parameter into the corresponding connection
 defined in `sql-connection-alist`, otherwise look into PARAMS.
 Look `sql-connection-alist` (part of SQL mode) for how to define
 database connections."
-  (if (assq :dbconnection params)
+  (if (and (assq :dbconnection params)
+	   (not (and (assq :database params) (eq name :database))))
       (let* ((dbconnection (cdr (assq :dbconnection params)))
              (name-mapping '((:dbhost . sql-server)
                              (:dbport . sql-port)
-- 
2.26.2

Re: [PATCH] ob-sql: Respect database param when using dbconnection

[Kyle Meyer]

Daniel Kraus writes:

> I use ob-sql with the :dbconnection param so I don't have my username and password in my org file.
> But often I don't want to use the default database from the dbconnection alist but
> rather specify it explicitly with :database.
> Attached is a patch that fixes this.

Thanks for the patch.

> Subject: [PATCH] ob-sql: Respect database param when using dbconnection
>
> ---
>  lisp/ob-sql.el | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)


Would you mind formatting your commit message as described at
<https://orgmode.org/worg/org-contribute.html>?  It'd also be good to
have an ORG-NEWS entry for this, I think.

> diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el
> index 7c359b988..d7a8bf0a0 100644
> --- a/lisp/ob-sql.el
> +++ b/lisp/ob-sql.el
> @@ -191,7 +191,8 @@ then look for the parameter into the corresponding connection
>  defined in `sql-connection-alist`, otherwise look into PARAMS.
>  Look `sql-connection-alist` (part of SQL mode) for how to define
>  database connections."
> -  (if (assq :dbconnection params)
> +  (if (and (assq :dbconnection params)
> +	   (not (and (assq :database params) (eq name :database))))
>        (let* ((dbconnection (cdr (assq :dbconnection params)))
>               (name-mapping '((:dbhost . sql-server)
>                               (:dbport . sql-port)

From what I can gather from your description, this looks reasonable.
I'm not an ob-sql user, so perhaps I missing something, but would it
make sense for any connection parameter to take precedence if explicitly
given in the source block header (i.e. something like the patch below)?
[+cc Stefano, who added the :dbconneciton feature.]


diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el
index 7c359b988..e7186938f 100644
--- a/lisp/ob-sql.el
+++ b/lisp/ob-sql.el
@@ -191,17 +191,17 @@ (defun org-babel-find-db-connection-param (params name)
 defined in `sql-connection-alist`, otherwise look into PARAMS.
 Look `sql-connection-alist` (part of SQL mode) for how to define
 database connections."
-  (if (assq :dbconnection params)
-      (let* ((dbconnection (cdr (assq :dbconnection params)))
-             (name-mapping '((:dbhost . sql-server)
-                             (:dbport . sql-port)
-                             (:dbuser . sql-user)
-                             (:dbpassword . sql-password)
-                             (:database . sql-database)))
-             (mapped-name (cdr (assq name name-mapping))))
-        (cadr (assq mapped-name
-                    (cdr (assoc dbconnection sql-connection-alist)))))
-    (cdr (assq name params))))
+  (or (cdr (assq name params))
+      (and (assq :dbconnection params)
+	   (let* ((dbconnection (cdr (assq :dbconnection params)))
+		  (name-mapping '((:dbhost . sql-server)
+				  (:dbport . sql-port)
+				  (:dbuser . sql-user)
+				  (:dbpassword . sql-password)
+				  (:database . sql-database)))
+		  (mapped-name (cdr (assq name name-mapping))))
+	     (cadr (assq mapped-name
+			 (cdr (assoc dbconnection sql-connection-alist))))))))
 
 (defun org-babel-execute:sql (body params)
   "Execute a block of Sql code with Babel.

Re: [PATCH] ob-sql: Respect database param when using dbconnection

[Stefano Rodighiero]

[-- Attachment #1: Type: text/plain, Size: 1276 bytes --]

On Mon, Jun 1, 2020 at 4:16 AM Kyle Meyer <kyle@kyleam.com> wrote:

Daniel Kraus writes:
>
> > I use ob-sql with the :dbconnection param so I don't have my username
> and password in my org file.
> > But often I don't want to use the default database from the dbconnection
> alist but
> > rather specify it explicitly with :database.
> > Attached is a patch that fixes this.
>

Thank you @Daniel


> […]
> From what I can gather from your description, this looks reasonable.
> I'm not an ob-sql user, so perhaps I missing something, but would it
> make sense for any connection parameter to take precedence if explicitly
> given in the source block header (i.e. something like the patch below)?
> [+cc Stefano, who added the :dbconneciton feature.]
>

I think it makes sense.

(I personally handle cases like those described by Daniel differently,
keeping distinct sql-connection-alist entries for each DB
param combination I might need, but I can imagine why someone
would want to "override" the database or the host params.
For port, user and password I have more difficulties imagining a
case where combinations of those params would need override,
but I think @Kyle's generic solution is better)

s.


-- 
www.stefanorodighiero.net

[-- Attachment #2: Type: text/html, Size: 2095 bytes --]

Re: [PATCH] ob-sql: Respect database param when using dbconnection

[Daniel Kraus]

[-- Attachment #1: Type: text/plain, Size: 318 bytes --]

Hi,

sorry for replying to a 2 year old thread.
Attached is a new patch with the change from Kyle and
I also added a news entry.
Not sure if the text for the news entry is any good,
or if there should be an example etc.
Feedback welcome.

Otherwise, the patch itself is not much and I use it daily.

Thanks,
  Daniel


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-sql.el-Respect-all-params-when-using-dbconnection.patch --]
[-- Type: text/x-patch, Size: 3383 bytes --]

From cd170dd691ba12ec81ef5c71db2868b33cd63ddf Mon Sep 17 00:00:00 2001
From: Daniel Kraus <daniel@kraus.my>
Date: Mon, 31 Oct 2022 07:52:09 +0100
Subject: [PATCH] ob-sql.el: Respect all params when using dbconnection

* etc/ORG-NEWS (Miscellaneous): Document change
* lisp/ob-sql.el (org-babel-find-db-connection-param): Make it
possible to overwrite parameters that are set from :dbconnection
---
 etc/ORG-NEWS   | 14 ++++++++++++++
 lisp/ob-sql.el | 24 ++++++++++++------------
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 6e875deb6..b542da34b 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -457,6 +457,20 @@ The new variable name is =org-plantuml-args=.  It now applies to both
 jar PlantUML file and executable.
 
 ** Miscellaneous
+*** SQL Babel ~:dbconnection~ parameter can be mixed with other SQL Babel parameters
+
+Before you could either specify SQL parameters like ~:dbhost~,
+~:dbuser~, ~:database~, etc or a ~:dbconnection~ parameter which looks
+up all other parameters from the ~sql-connection-alist~ variable.  Now
+it's possible to specify a ~:dbconnection~ and additionally other
+parameters that will add or overwrite the parameters coming from
+~sql-connection-alist~.
+
+E.g. if you have a connection in your ~sql-connection-alist~ to a
+server that has many databases, you don't need an entry for every
+database but instead can just specify ~:database~ next to your
+~:dbconnection~ parameter.
+
 *** Post-processing code blocks can return an empty list
 
 When the result of a regular code block is nil, then that was already
diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el
index d1256bf83..626d595c9 100644
--- a/lisp/ob-sql.el
+++ b/lisp/ob-sql.el
@@ -222,18 +222,18 @@ then look for the parameter into the corresponding connection
 defined in `sql-connection-alist', otherwise look into PARAMS.
 See `sql-connection-alist' (part of SQL mode) for how to define
 database connections."
-  (if (assq :dbconnection params)
-      (let* ((dbconnection (cdr (assq :dbconnection params)))
-             (name-mapping '((:dbhost . sql-server)
-                             (:dbport . sql-port)
-                             (:dbuser . sql-user)
-                             (:dbpassword . sql-password)
-                             (:dbinstance . sql-dbinstance)
-                             (:database . sql-database)))
-             (mapped-name (cdr (assq name name-mapping))))
-        (cadr (assq mapped-name
-                    (cdr (assoc dbconnection sql-connection-alist)))))
-    (cdr (assq name params))))
+  (or (cdr (assq name params))
+      (and (assq :dbconnection params)
+           (let* ((dbconnection (cdr (assq :dbconnection params)))
+                  (name-mapping '((:dbhost . sql-server)
+                                  (:dbport . sql-port)
+                                  (:dbuser . sql-user)
+                                  (:dbpassword . sql-password)
+                                  (:dbinstance . sql-dbinstance)
+                                  (:database . sql-database)))
+                  (mapped-name (cdr (assq name name-mapping))))
+             (cadr (assq mapped-name
+                         (cdr (assoc dbconnection sql-connection-alist))))))))
 
 (defun org-babel-execute:sql (body params)
   "Execute a block of Sql code with Babel.
-- 
2.38.1


[-- Attachment #3: Type: text/plain, Size: 1349 bytes --]


Stefano Rodighiero <stefano.rodighiero@gmail.com> writes:

> On Mon, Jun 1, 2020 at 4:16 AM Kyle Meyer <kyle@kyleam.com> wrote:
>
> Daniel Kraus writes:
>>
>> > I use ob-sql with the :dbconnection param so I don't have my username
>> and password in my org file.
>> > But often I don't want to use the default database from the dbconnection
>> alist but
>> > rather specify it explicitly with :database.
>> > Attached is a patch that fixes this.
>>
>
> Thank you @Daniel
>
>
>> […]
>> From what I can gather from your description, this looks reasonable.
>> I'm not an ob-sql user, so perhaps I missing something, but would it
>> make sense for any connection parameter to take precedence if explicitly
>> given in the source block header (i.e. something like the patch below)?
>> [+cc Stefano, who added the :dbconneciton feature.]
>>
>
> I think it makes sense.
>
> (I personally handle cases like those described by Daniel differently,
> keeping distinct sql-connection-alist entries for each DB
> param combination I might need, but I can imagine why someone
> would want to "override" the database or the host params.
> For port, user and password I have more difficulties imagining a
> case where combinations of those params would need override,
> but I think @Kyle's generic solution is better)
>
> s.

Re: [PATCH] ob-sql: Respect database param when using dbconnection

[Ihor Radchenko]

Daniel Kraus <daniel@kraus.my> writes:

> Hi,
>
> sorry for replying to a 2 year old thread.
> Attached is a new patch with the change from Kyle and
> I also added a news entry.
> Not sure if the text for the news entry is any good,
> or if there should be an example etc.
> Feedback welcome.
>
> Otherwise, the patch itself is not much and I use it daily.

Thanks!
Applied onto main with minor amendments to the commit message (I added
"." at the end of the sentences).
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=fbbc8b55cf3a39c85f8576321ec560baf4d2331a

P.S. ob-sql currently have no maintainer. Do you want to volunteer?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] ob-sql: Respect database param when using dbconnection

[Daniel Kraus]

Ihor Radchenko <yantar92@posteo.net> writes:

>> Otherwise, the patch itself is not much and I use it daily.
>
> Thanks!
> Applied onto main with minor amendments to the commit message (I added
> "." at the end of the sentences).
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=fbbc8b55cf3a39c85f8576321ec560baf4d2331a

Thanks.


> P.S. ob-sql currently have no maintainer. Do you want to volunteer?

Hmm, ob-sql is the the babel module I use most often.
I don't think I'll have enough time to make bigger changes etc,
but from what I know that's not necessary or expected.
I'm certainly willing to look at patches, answer bug reports
and join discussion about ob-sql.

tl;dr you can add me as a maintainer ;)

Cheers,
  Daniel

Re: [PATCH] ob-sql: Respect database param when using dbconnection

[Ihor Radchenko]

Daniel Kraus <daniel@kraus.my> writes:

>> P.S. ob-sql currently have no maintainer. Do you want to volunteer?
>
> Hmm, ob-sql is the the babel module I use most often.
> I don't think I'll have enough time to make bigger changes etc,
> but from what I know that's not necessary or expected.
> I'm certainly willing to look at patches, answer bug reports
> and join discussion about ob-sql.
>
> tl;dr you can add me as a maintainer ;)

Bastien, could you please add Daniel as the maintainer of lisp/ob-sql.el?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] ob-sql: Respect database param when using dbconnection

[Bastien Guerry]

Ihor Radchenko <yantar92@posteo.net> writes:

> Bastien, could you please add Daniel as the maintainer of
> lisp/ob-sql.el?

Done in bd468136d.  Thanks Daniel!

-- 
 Bastien