[-- Attachment #1.1: Type: text/plain, Size: 129 bytes --] ob-sql.el does not respect the value of sql-postgres-program, thus causing problem when running on Windows, etc. Patch attached. [-- Attachment #1.2: Type: text/html, Size: 192 bytes --] [-- Attachment #2: 0001-Respect-the-value-of-sql-postgres-program-as-opposed.patch --] [-- Type: application/octet-stream, Size: 1062 bytes --] From 237c5a1d46675de78ccc32bc9a62cc1fd5300383 Mon Sep 17 00:00:00 2001 From: Alan Light <lightalan@gmail.com> Date: Tue, 8 Dec 2020 11:27:34 -0500 Subject: [PATCH] Respect the value of sql-postgres-program as opposed to having psql hard-coded --- lisp/org/ob-sql.el | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lisp/org/ob-sql.el b/lisp/org/ob-sql.el index 59cf19568e..8eca769d02 100644 --- a/lisp/org/ob-sql.el +++ b/lisp/org/ob-sql.el @@ -237,11 +237,12 @@ This function is called by `org-babel-execute-src-block'." (org-babel-process-file-name in-file) (org-babel-process-file-name out-file))) (`postgresql (format - "%spsql --set=\"ON_ERROR_STOP=1\" %s -A -P \ + "%s%s --set=\"ON_ERROR_STOP=1\" %s -A -P \ footer=off -F \"\t\" %s -f %s -o %s %s" (if dbpassword (format "PGPASSWORD=%s " dbpassword) "") + (or sql-postgres-program "psql") (if colnames-p "" "-t") (org-babel-sql-dbstring-postgresql dbhost dbport dbuser database) -- 2.25.1
Hi Alan, Alan Light <lightalan@gmail.com> writes: > ob-sql.el does not respect the value of sql-postgres-program, thus > causing problem when running on Windows, etc. > Patch attached. Thanks. Can someone confirm the patch is good? Alan, can you check how to submit a patch with a changelog on this page: https://orgmode.org/worg/org-contribute.html and resubmit it? Best, -- Bastien
[-- Attachment #1: Type: text/plain, Size: 567 bytes --] Sorry, I don't understand. That's what I did. The patch was attached to my email. On Sat, Dec 12, 2020 at 1:32 PM Bastien <bzg@gnu.org> wrote: > Hi Alan, > > Alan Light <lightalan@gmail.com> writes: > > > ob-sql.el does not respect the value of sql-postgres-program, thus > > causing problem when running on Windows, etc. > > Patch attached. > > Thanks. Can someone confirm the patch is good? > > Alan, can you check how to submit a patch with a changelog on this > page: https://orgmode.org/worg/org-contribute.html and resubmit it? > > Best, > > -- > Bastien > [-- Attachment #2: Type: text/html, Size: 1069 bytes --]
Alan Light <lightalan@gmail.com> writes:
> Sorry, I don't understand. That's what I did. The patch was attached to my
> email.
You need to add patch description, not just title. Something like:
* ob-sql.el (org-babel-execute:sql): Use `sql-postgres-program' as
postgresql executable (instead of psql) when defined.
Best,
Ihor
[-- Attachment #1.1: Type: text/plain, Size: 459 bytes --] New patch file attached. On Sat, Dec 12, 2020 at 7:14 PM Ihor Radchenko <yantar92@gmail.com> wrote: > Alan Light <lightalan@gmail.com> writes: > > > Sorry, I don't understand. That's what I did. The patch was attached to > my > > email. > > You need to add patch description, not just title. Something like: > > * ob-sql.el (org-babel-execute:sql): Use `sql-postgres-program' as > postgresql executable (instead of psql) when defined. > > Best, > Ihor > [-- Attachment #1.2: Type: text/html, Size: 869 bytes --] [-- Attachment #2: 0002-ob-sql.el-ob-sql.el-org-babel-execute-sql-Use-sql-po.patch --] [-- Type: application/octet-stream, Size: 1036 bytes --] From 62329424577d9baa40cca3c98951c4b65a095002 Mon Sep 17 00:00:00 2001 From: Alan Light <lightalan@gmail.com> Date: Sat, 12 Dec 2020 19:26:40 -0500 Subject: [PATCH 2/2] ob-sql.el: ob-sql.el (org-babel-execute:sql): Use `sql-postgres-program' as postgresql executable (instead of psql) when defined * ob-sql.el (org-babel-execute:sql): Use `sql-postgres-program' as postgresql executable (instead of psql) when defined. psql will be the default --- lisp/org/ob-sql.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lisp/org/ob-sql.el b/lisp/org/ob-sql.el index 8eca769d02..f46c6c1c6a 100644 --- a/lisp/org/ob-sql.el +++ b/lisp/org/ob-sql.el @@ -242,7 +242,7 @@ footer=off -F \"\t\" %s -f %s -o %s %s" (if dbpassword (format "PGPASSWORD=%s " dbpassword) "") - (or sql-postgres-program "psql") + (or sql-postgres-program "psql") ;; default is psql (if colnames-p "" "-t") (org-babel-sql-dbstring-postgresql dbhost dbport dbuser database) -- 2.25.1
> when defined. psql will For future: You need to use 2 spaces after sentence end ^_^. As I understand, these NEWS entries must have specific formatting to be harvested automatically into NEWS file. So, patches should comply with all the requirements listed in https://orgmode.org/worg/org-contribute.html Best, Ihor Alan Light <lightalan@gmail.com> writes: > New patch file attached.
It looks like the two patches are sequential, there should probably be a rebase into a single patch. I would remove the comment in the second patch because it is a single command to jump to the default value and it might change in the future, so no reason to put it in a comment. One way to communicate about the source of that variable is to include (require 'sql) in ob-sql which is likely needed anyway due to the fact that sql-postgres-program is defined by sql.el and there will be a byte compiler error because the variable is undefined. Having the value of "psql" present as a backup in case sql-postgres-program is somehow nil seems reasonable. Best, Tom
Thanks for the patch, and others for the review/feedback. Alan Light writes: > Subject: [PATCH 2/2] ob-sql.el: ob-sql.el (org-babel-execute:sql): Use > `sql-postgres-program' as postgresql executable (instead of psql) when > defined > > * ob-sql.el (org-babel-execute:sql): Use `sql-postgres-program' as > postgresql executable (instead of psql) when defined. psql will be the default I've squash this patch, which as Tom notes is a fixup on top of the initial one, into the first. I've also tweaked the commit message a bit, including adding TINYCHANGE. https://orgmode.org/worg/org-contribute.html#commit-messages > --- > lisp/org/ob-sql.el | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lisp/org/ob-sql.el b/lisp/org/ob-sql.el > index 8eca769d02..f46c6c1c6a 100644 > --- a/lisp/org/ob-sql.el > +++ b/lisp/org/ob-sql.el > @@ -242,7 +242,7 @@ footer=off -F \"\t\" %s -f %s -o %s %s" > (if dbpassword > (format "PGPASSWORD=%s " dbpassword) > "") > - (or sql-postgres-program "psql") > + (or sql-postgres-program "psql") ;; default is psql As Tom noted, the use of sql-postgres-program here leads to a byte-compiler warning. It probably makes sense to just require sql.el in the file, but for now I've guarded this with a bound-and-true-p. Pushed (862048d4c). Thanks again.