GNU bug report logs - #76025
[PATCH 1/1] * lisp/progmodes/sql.el: login without prompting

Previous Next

Package: emacs;

Reported by: Phil Estival <pe <at> 7d.nz>

Date: Mon, 3 Feb 2025 05:20:02 UTC

Severity: wishlist

Tags: patch

To reply to this bug, email your comments to 76025 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to yantar92 <at> posteo.net, bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Mon, 03 Feb 2025 05:20:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Phil Estival <pe <at> 7d.nz>:
New bug report received and forwarded. Copy sent to yantar92 <at> posteo.net, bug-gnu-emacs <at> gnu.org. (Mon, 03 Feb 2025 05:20:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Phil Estival <pe <at> 7d.nz>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH 1/1] * lisp/progmodes/sql.el: login without prompting
Date: Sun, 02 Feb 2025 17:31:08 +0100
[Message part 1 (text/plain, inline)]
Tags: patch


This is a patch of `sql-product-interactive' in order to connect without
prompting for or asking to confirm connection parameters, using ones
already set and stored in `sql-database', `sql-server', -user, etc.

In GNU Emacs 30.0.50
Repository revision: f2bccae22bd47a2e7e0937b78ea06131711b935a
Repository branch: master

[0001-lisp-progmodes-sql.el-login-without-prompting.patch (text/patch, attachment)]
[Message part 3 (text/plain, inline)]
--
Phil Estival

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Thu, 06 Feb 2025 19:13:02 GMT) Full text and rfc822 format available.

Message #8 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: Phil Estival <pe <at> 7d.nz>
Cc: 76025 <at> debbugs.gnu.org, Michael Mauger <michael <at> mauger.com>
Subject: Re: bug#76025: [PATCH 1/1] * lisp/progmodes/sql.el: login without
 prompting
Date: Thu, 06 Feb 2025 19:14:50 +0000
Phil Estival <pe <at> 7d.nz> writes:

> This is a patch of `sql-product-interactive' in order to connect without
> prompting for or asking to confirm connection parameters, using ones
> already set and stored in `sql-database', `sql-server', -user, etc.

For context, we want this feature to improve Org babel's sql library
(ob-sql) where we need to run session programmatically. See
https://list.orgmode.org/orgmode/646f7d12-a3d1-4a7c-83e2-5eecd7ca6817 <at> 7d.nz/

CCing sql.el maintainer.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Fri, 07 Feb 2025 02:43:02 GMT) Full text and rfc822 format available.

Message #11 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Michael Mauger <mmauger <at> protonmail.com>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: Phil Estival <pe <at> 7d.nz>, 76025 <at> debbugs.gnu.org,
 Michael Mauger <michael <at> mauger.com>
Subject: Re: bug#76025: [PATCH 1/1] * lisp/progmodes/sql.el: login without
 prompting
Date: Fri, 07 Feb 2025 02:42:19 +0000
On Thursday, February 6th, 2025 at 7:12 PM, Ihor Radchenko <yantar92 <at> posteo.net> wrote:

> Phil Estival pe <at> 7d.nz writes:
> 
> > This is a patch of `sql-product-interactive' in order to connect without prompting for or asking to confirm connection parameters, using ones already set and stored in` sql-database', `sql-server', -user, etc.
> 
> 
> For context, we want this feature to improve Org babel's sql library
> (ob-sql) where we need to run session programmatically. See
> https://list.orgmode.org/orgmode/646f7d12-a3d1-4a7c-83e2-5eecd7ca6817 <at> 7d.nz/
> 
> CCing sql.el maintainer.
> 

Okay, a lot here... 🤔

For my context, how are ob-sql "sessions" different from what is provided by `sql-product-interactive` buffers? I see you avoiding `sql-product-interactive` buffers but the result is that you are duplicating a lot of logic already present in `sql.el`. I have used babel to drive a sqlite3 in-memory database without the ob-sql machinery, so I'm wondering how I must warp my old batch card deck way of thinking to understand the problem you are trying to solve.

I do see that you are supporting "engines" not present in `sql.el`; have interactive support for these engines been made available as ELPA/MELPA/... modules? Should they be?

I also see hardcoding of sql command processor cli names; why not use the name already customized in sql.el?

I am very interested in supporting this effort within sql.el if I can. I inherited sql.el a couple of decades ago and other than adding support for using multiple dialects within the same Emacs instance, the feature set is pretty bare-bones. I have some time rn to focus on this, so if someone has the time/inclination to expand my vision, I'd appreciate it.

-- 
MICHAEL <at> MAUGER.COM // FSF and SFConservancy // GNU Emacs sql.el maintainer




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Fri, 07 Feb 2025 07:15:02 GMT) Full text and rfc822 format available.

Message #14 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: pe <at> 7d.nz, 76025 <at> debbugs.gnu.org, michael <at> mauger.com
Subject: Re: bug#76025: [PATCH 1/1] * lisp/progmodes/sql.el: login without
 prompting
Date: Fri, 07 Feb 2025 09:14:28 +0200
> Cc: 76025 <at> debbugs.gnu.org, Michael Mauger <michael <at> mauger.com>
> From: Ihor Radchenko <yantar92 <at> posteo.net>
> Date: Thu, 06 Feb 2025 19:14:50 +0000
> 
> Phil Estival <pe <at> 7d.nz> writes:
> 
> > This is a patch of `sql-product-interactive' in order to connect without
> > prompting for or asking to confirm connection parameters, using ones
> > already set and stored in `sql-database', `sql-server', -user, etc.
> 
> For context, we want this feature to improve Org babel's sql library
> (ob-sql) where we need to run session programmatically. See
> https://list.orgmode.org/orgmode/646f7d12-a3d1-4a7c-83e2-5eecd7ca6817 <at> 7d.nz/
> 
> CCing sql.el maintainer.

Thanks.

From my POV, this change needs a NEWS entry, and I think Phil needs to
do the legal paperwork of assigning the copyright to the FSF (I can
send the form and the instructions if that's okay with Phil).




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Sun, 09 Feb 2025 13:03:01 GMT) Full text and rfc822 format available.

Message #17 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Phil Estival <pe <at> 7d.nz>
To: Michael Mauger <mmauger <at> protonmail.com>
Cc: Daniel Kraus <daniel <at> kraus.my>, 76025 <at> debbugs.gnu.org,
 Ihor Radchenko <yantar92 <at> posteo.net>, Michael Mauger <michael <at> mauger.com>
Subject: Re: bug#76025: [PATCH 1/1] * lisp/progmodes/sql.el: login without
 prompting
Date: Sun, 09 Feb 2025 14:01:51 +0100
* [2025-02-07 02:42 +0000] Michael Mauger <mmauger <at> protonmail.com>:
> On Thursday, February 6th, 2025 at 7:12 PM, Ihor Radchenko <yantar92 <at> posteo.net> wrote:
>> Phil Estival pe <at> 7d.nz writes:
>>
>> > This is a patch of `sql-product-interactive' in order to connect
>> > without prompting for or asking to confirm connection parameters,
>> > using ones already set and stored in` sql-database', `sql-server',
>> > -user, etc.
>>
>>
>> For context, we want this feature to improve Org babel's sql library
>> (ob-sql) where we need to run session programmatically. See
>> https://list.orgmode.org/orgmode/646f7d12-a3d1-4a7c-83e2-5eecd7ca6817 <at> 7d.nz/
>>
>> CCing sql.el maintainer.
>>


Hello Michael,

> For my context, how are ob-sql "sessions" different from what is
> provided by `sql-product-interactive` buffers?

Similar to other channels org-babel can communicate through, an ob-sql
session (info "(org) Environment of a Code Block") can be characterized
as a channel to an interpreter process through a comint shell buffer,
shared among source blocks in a notebook.

> I see you avoiding `sql-product-interactive` buffers but the result is
> that you are duplicating a lot of logic already present in `sql.el`.

This duplicated logic is the connection to the database server, modified
to avoid the prompts asking to confirm the parameters, except if one is
missing, since these parameters are already written in fields
describing, above the sql commands, what the session is: a named
connection to a database.  These parameters can be variables,
expressions, global to a document or locally scoped to a section, a
paragraph or a single code block.

Bypassing this prompts is necessary to automate the tests of the ob-sql
machinery.

If completion, coloring or additional interactions with the shell are
needed, then `sql-interactive-mode' could be set in the shell ob-sql is
communicating with.  "Could", because, this connection's function was
"borrowed" from sql.el with these slight modifications: not prompting
for parameters, no syntax coloring or completion, which make sense if
no direct interaction is expected in such buffers.

Ihor identified the connection's function as a duplication, and proposed
we streamline the code by relying on the existing function in
sql.el. Now ob-sql sessions do call `sql-product-interactive', hence
this proposal for a patch skipping the prompts.

Asking additional confirmations could be fine as a safeguard, but as I
said /supra/, it's an obstacle for automation and not a necessity when
describing a database in a notebook since these information are already
present in the environment of code blocks.

> I have used babel to drive a sqlite3 in-memory database without the
> ob-sql machinery, so I'm wondering how I must warp my old batch card
> deck way of thinking to understand the problem you are trying to
> solve.

One may need of a few stateful variables to designate schema or to give
names to roles, and so on.  These variables exists in the environment of
the SQL shell.  One way to apply changes of values is to reset the db,
before providing a new environment to a next run of the entire program.
But we can also change those values from inside a running environment
and recall only the code blocks that refers to these variables.  This is
what a "session" covers.

Org-mode, as a programming notebook is a convenient way to work on small
units of code, the blocks, organized in sections, and to combine their
execution.  In many situations, it's more operable and more maneuverable
than the files and folders we are used to think in, and that, in many
ways, we still conform to, due to their presence rooted at the core of
our systems.

As for the in-memory sqlite3 DB, it is an appropriate candidate to test
the correctness of the ob-sql sessions behavior: it's a standard
lightweight package assumed to be /de facto/ available on our operating
systems.

> I do see that you are supporting "engines" not present in `sql.el`;
> have interactive support for these engines been made available as
> ELPA/MELPA/... modules? Should they be?

Which ones? CC'ing Daniel to possibly get more advice on the matter.

> I also see hardcoding of sql command processor cli names; why not use
> the name already customized in sql.el?

Because sql.el was not a requirement to ob-sql.el, but you're right,
when it will be one, we can bind these to the sql-[db]-program. The
probability for someone to need two different cli programs, one in an
interactive shell and a different one in a notebook, for a same database
system is very low, and in such case he or she would still be capable to
set local values to adapt the situation.

> I am very interested in supporting this effort within sql.el if I
> can. I inherited sql.el a couple of decades ago and other than adding
> support for using multiple dialects within the same Emacs instance,
> the feature set is pretty bare-bones. I have some time rn to focus on
> this, so if someone has the time/inclination to expand my vision, I'd
> appreciate it.

I will add that a single interface to all these DB systems helps to
underline common requirements expected from these sql cli.  The case of
table formatting on MariaDB, configurable only at startup through a
command line parameter -- and not as a state variable -- is one such
requirement.

Thanks.

--
Phil Estival




Severity set to 'wishlist' from 'normal' Request was from Stefan Kangas <stefankangas <at> gmail.com> to control <at> debbugs.gnu.org. (Tue, 11 Feb 2025 07:14:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Sat, 15 Feb 2025 16:27:02 GMT) Full text and rfc822 format available.

Message #22 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: Phil Estival <pe <at> 7d.nz>
Cc: Michael Mauger <mmauger <at> protonmail.com>,
 Org Mode List <emacs-orgmode <at> gnu.org>, 76025 <at> debbugs.gnu.org
Subject: Re: [PATCH] ob-sql: session + sql.el w/o prompt
Date: Sat, 15 Feb 2025 16:25:25 +0000
Ihor Radchenko <yantar92 <at> posteo.net> writes:

> May you please post the patch on Emacs bug tracker? (M-x
> submit-emacs-patch) and X-Debbugs-CC me?
> That way, we get the Emacs maintainers involved into the discussion.

The patch is in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=76025

There are no major objections to the patch other than the maintainer
being interested to coordinate ob-sql improvements and sql.el
changes.

I am CCing the feature request thread so that we can discuss everything
together.

May you please update your latest patch for ob-sql.el, converting it
into (1) patch to sql.el; (2) patch for ob-sql.el that assumes changes
to sql.el?

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Tue, 18 Feb 2025 20:48:03 GMT) Full text and rfc822 format available.

Message #25 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Phil Estival <pe <at> 7d.nz>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76025 <at> debbugs.gnu.org, Ihor Radchenko <yantar92 <at> posteo.net>,
 michael <at> mauger.com
Subject: Re: bug#76025: [PATCH 1/1] * lisp/progmodes/sql.el: login without
 prompting
Date: Tue, 18 Feb 2025 21:47:23 +0100
* [2025-02-07 09:14 +0200] Eli Zaretskii <eliz <at> gnu.org>:
>> Cc: 76025 <at> debbugs.gnu.org, Michael Mauger <michael <at> mauger.com>
>> From: Ihor Radchenko <yantar92 <at> posteo.net>
>> Date: Thu, 06 Feb 2025 19:14:50 +0000
>>
>> Phil Estival <pe <at> 7d.nz> writes:
>>
>> > This is a patch of `sql-product-interactive' in order to connect without
>> > prompting for or asking to confirm connection parameters, using ones
>> > already set and stored in `sql-database', `sql-server', -user, etc.
>>
>> For context, we want this feature to improve Org babel's sql library
>> (ob-sql) where we need to run session programmatically. See
>> https://list.orgmode.org/orgmode/646f7d12-a3d1-4a7c-83e2-5eecd7ca6817 <at> 7d.nz/
>>
>> CCing sql.el maintainer.
>
> Thanks.
>
> From my POV, this change needs a NEWS entry, and I think Phil needs to
> do the legal paperwork of assigning the copyright to the FSF (I can
> send the form and the instructions if that's okay with Phil).

I asked assign <at> gnu.org for a pgp pub key last Wednesday
but no response so far. Is there any?

--
Phil Estival




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Wed, 19 Feb 2025 12:12:02 GMT) Full text and rfc822 format available.

Message #28 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Phil Estival <pe <at> 7d.nz>
Cc: 76025 <at> debbugs.gnu.org, yantar92 <at> posteo.net, michael <at> mauger.com
Subject: Re: bug#76025: [PATCH 1/1] * lisp/progmodes/sql.el: login without
 prompting
Date: Wed, 19 Feb 2025 14:11:25 +0200
> From: Phil Estival <pe <at> 7d.nz>
> Cc: Ihor Radchenko <yantar92 <at> posteo.net>,  76025 <at> debbugs.gnu.org,
>   michael <at> mauger.com
> Date: Tue, 18 Feb 2025 21:47:23 +0100
> 
> * [2025-02-07 09:14 +0200] Eli Zaretskii <eliz <at> gnu.org>:
> >> Cc: 76025 <at> debbugs.gnu.org, Michael Mauger <michael <at> mauger.com>
> >> From: Ihor Radchenko <yantar92 <at> posteo.net>
> >> Date: Thu, 06 Feb 2025 19:14:50 +0000
> >>
> >> Phil Estival <pe <at> 7d.nz> writes:
> >>
> >> > This is a patch of `sql-product-interactive' in order to connect without
> >> > prompting for or asking to confirm connection parameters, using ones
> >> > already set and stored in `sql-database', `sql-server', -user, etc.
> >>
> >> For context, we want this feature to improve Org babel's sql library
> >> (ob-sql) where we need to run session programmatically. See
> >> https://list.orgmode.org/orgmode/646f7d12-a3d1-4a7c-83e2-5eecd7ca6817 <at> 7d.nz/
> >>
> >> CCing sql.el maintainer.
> >
> > Thanks.
> >
> > From my POV, this change needs a NEWS entry, and I think Phil needs to
> > do the legal paperwork of assigning the copyright to the FSF (I can
> > send the form and the instructions if that's okay with Phil).
> 
> I asked assign <at> gnu.org for a pgp pub key last Wednesday
> but no response so far. Is there any?

I don't know, I never send any encrypted messages to them.  In any
case, please wait for a few more days, and then ping them and CC me.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Tue, 18 Mar 2025 18:00:03 GMT) Full text and rfc822 format available.

Message #31 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: Phil Estival <pe <at> 7d.nz>
Cc: 76025 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, michael <at> mauger.com
Subject: Re: bug#76025: [PATCH 1/1] * lisp/progmodes/sql.el: login without
 prompting
Date: Tue, 18 Mar 2025 17:58:56 +0000
Phil Estival <pe <at> 7d.nz> writes:

>> From my POV, this change needs a NEWS entry, and I think Phil needs to
>> do the legal paperwork of assigning the copyright to the FSF (I can
>> send the form and the instructions if that's okay with Phil).
>
> I asked assign <at> gnu.org for a pgp pub key last Wednesday
> but no response so far. Is there any?

May I know if there is any progress?

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Tue, 18 Mar 2025 20:49:04 GMT) Full text and rfc822 format available.

Message #34 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: pe <at> 7d.nz, 76025 <at> debbugs.gnu.org, michael <at> mauger.com
Subject: Re: bug#76025: [PATCH 1/1] * lisp/progmodes/sql.el: login without
 prompting
Date: Tue, 18 Mar 2025 22:48:28 +0200
> From: Ihor Radchenko <yantar92 <at> posteo.net>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 76025 <at> debbugs.gnu.org, michael <at> mauger.com
> Date: Tue, 18 Mar 2025 17:58:56 +0000
> 
> Phil Estival <pe <at> 7d.nz> writes:
> 
> >> From my POV, this change needs a NEWS entry, and I think Phil needs to
> >> do the legal paperwork of assigning the copyright to the FSF (I can
> >> send the form and the instructions if that's okay with Phil).
> >
> > I asked assign <at> gnu.org for a pgp pub key last Wednesday
> > but no response so far. Is there any?
> 
> May I know if there is any progress?

Not that I know of, no.

I also didn't see any correspondence between Phil and the copyright
clerk, although usually the copyright clerk CC's me on his responses.

Phil, did you send the form to the FSF?  Or are you still waiting for
their PGP key?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Mon, 07 Apr 2025 12:01:02 GMT) Full text and rfc822 format available.

Message #37 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Phil Estival <pe <at> 7d.nz>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76025 <at> debbugs.gnu.org, Ihor Radchenko <yantar92 <at> posteo.net>,
 michael <at> mauger.com
Subject: Re: bug#76025: [PATCH 1/1] * lisp/progmodes/sql.el: login without
 prompting
Date: Mon, 07 Apr 2025 13:59:47 +0200
* [2025-03-18 22:48 +0200] Eli Zaretskii <eliz <at> gnu.org>:
>> From: Ihor Radchenko <yantar92 <at> posteo.net>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>, 76025 <at> debbugs.gnu.org, michael <at> mauger.com
>> Date: Tue, 18 Mar 2025 17:58:56 +0000
>>
>> Phil Estival <pe <at> 7d.nz> writes:
>>
>> >> From my POV, this change needs a NEWS entry, and I think Phil needs to
>> >> do the legal paperwork of assigning the copyright to the FSF (I can
>> >> send the form and the instructions if that's okay with Phil).
>> >
>> > I asked assign <at> gnu.org for a pgp pub key last Wednesday
>> > but no response so far. Is there any?
>>
>> May I know if there is any progress?
>
> Not that I know of, no.
>
> I also didn't see any correspondence between Phil and the copyright
> clerk, although usually the copyright clerk CC's me on his responses.
>
> Phil, did you send the form to the FSF?  Or are you still waiting for
> their PGP key?

I'm writing to them again to ask if there is one.

--
Phil Estival




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Tue, 15 Apr 2025 16:31:02 GMT) Full text and rfc822 format available.

Message #40 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: Phil Estival <pe <at> 7d.nz>
Cc: Org Mode List <emacs-orgmode <at> gnu.org>,
 Michael Mauger <mmauger <at> protonmail.com>, 76025 <at> debbugs.gnu.org
Subject: Re: bug#76025: [PATCH] ob-sql: session + sql.el w/o prompt
Date: Tue, 15 Apr 2025 16:29:07 +0000
Ihor Radchenko <yantar92 <at> posteo.net> writes:

> May you please update your latest patch for ob-sql.el, converting it
> into (1) patch to sql.el; (2) patch for ob-sql.el that assumes changes
> to sql.el?

It has been a while since the last message in this thread.
Phil, do you need any help on this?

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Wed, 16 Apr 2025 16:25:08 GMT) Full text and rfc822 format available.

Message #43 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Phil Estival <pe <at> 7d.nz>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: Org Mode List <emacs-orgmode <at> gnu.org>,
 Michael Mauger <mmauger <at> protonmail.com>, 76025 <at> debbugs.gnu.org
Subject: Re: bug#76025: [PATCH] ob-sql: session + sql.el w/o prompt
Date: Wed, 16 Apr 2025 18:24:16 +0200
* [2025-04-15 16:29 +0000] Ihor Radchenko <yantar92 <at> posteo.net>:
> Ihor Radchenko <yantar92 <at> posteo.net> writes:
>
>> May you please update your latest patch for ob-sql.el, converting it
>> into (1) patch to sql.el; (2) patch for ob-sql.el that assumes changes
>> to sql.el?
>
> It has been a while since the last message in this thread.
> Phil, do you need any help on this?

I'm on it. I planned last monday to provide a release of this
contribution this week and the next one. I've been using it as a work
configuration lately and brought a few changes.

--
Phil Estival




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Sat, 19 Apr 2025 13:57:03 GMT) Full text and rfc822 format available.

Message #46 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Phil Estival <pe <at> 7d.nz>
Cc: 76025 <at> debbugs.gnu.org, yantar92 <at> posteo.net, michael <at> mauger.com
Subject: Re: bug#76025: [PATCH 1/1] * lisp/progmodes/sql.el: login without
 prompting
Date: Sat, 19 Apr 2025 16:56:14 +0300
> From: Phil Estival <pe <at> 7d.nz>
> Cc: Ihor Radchenko <yantar92 <at> posteo.net>,  76025 <at> debbugs.gnu.org,
>   michael <at> mauger.com
> Date: Mon, 07 Apr 2025 13:59:47 +0200
> 
> * [2025-03-18 22:48 +0200] Eli Zaretskii <eliz <at> gnu.org>:
> >> From: Ihor Radchenko <yantar92 <at> posteo.net>
> >> Cc: Eli Zaretskii <eliz <at> gnu.org>, 76025 <at> debbugs.gnu.org, michael <at> mauger.com
> >> Date: Tue, 18 Mar 2025 17:58:56 +0000
> >>
> >> Phil Estival <pe <at> 7d.nz> writes:
> >>
> >> >> From my POV, this change needs a NEWS entry, and I think Phil needs to
> >> >> do the legal paperwork of assigning the copyright to the FSF (I can
> >> >> send the form and the instructions if that's okay with Phil).
> >> >
> >> > I asked assign <at> gnu.org for a pgp pub key last Wednesday
> >> > but no response so far. Is there any?
> >>
> >> May I know if there is any progress?
> >
> > Not that I know of, no.
> >
> > I also didn't see any correspondence between Phil and the copyright
> > clerk, although usually the copyright clerk CC's me on his responses.
> >
> > Phil, did you send the form to the FSF?  Or are you still waiting for
> > their PGP key?
> 
> I'm writing to them again to ask if there is one.

Any progress with this issue?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Mon, 28 Apr 2025 15:54:05 GMT) Full text and rfc822 format available.

Message #49 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Phil Estival <pe <at> 7d.nz>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76025 <at> debbugs.gnu.org, yantar92 <at> posteo.net, michael <at> mauger.com
Subject: Re: bug#76025: [PATCH 1/1] * lisp/progmodes/sql.el: login without
 prompting
Date: Mon, 28 Apr 2025 17:52:54 +0200
* [2025-04-19 16:56 +0300] Eli Zaretskii <eliz <at> gnu.org>:
>> From: Phil Estival <pe <at> 7d.nz>
>> Cc: Ihor Radchenko <yantar92 <at> posteo.net>,  76025 <at> debbugs.gnu.org,
>>   michael <at> mauger.com
>> Date: Mon, 07 Apr 2025 13:59:47 +0200
>>
>> * [2025-03-18 22:48 +0200] Eli Zaretskii <eliz <at> gnu.org>:
>> >> From: Ihor Radchenko <yantar92 <at> posteo.net>
>> >> Cc: Eli Zaretskii <eliz <at> gnu.org>, 76025 <at> debbugs.gnu.org, michael <at> mauger.com
>> >> Date: Tue, 18 Mar 2025 17:58:56 +0000
>> >>
>> >> Phil Estival <pe <at> 7d.nz> writes:
>> >>
>> >> >> From my POV, this change needs a NEWS entry, and I think Phil needs to
>> >> >> do the legal paperwork of assigning the copyright to the FSF (I can
>> >> >> send the form and the instructions if that's okay with Phil).
>> >> >
>> >> > I asked assign <at> gnu.org for a pgp pub key last Wednesday
>> >> > but no response so far. Is there any?
>> >>
>> >> May I know if there is any progress?
>> >
>> > Not that I know of, no.
>> >
>> > I also didn't see any correspondence between Phil and the copyright
>> > clerk, although usually the copyright clerk CC's me on his responses.
>> >
>> > Phil, did you send the form to the FSF?  Or are you still waiting for
>> > their PGP key?
>>
>> I'm writing to them again to ask if there is one.
>
> Any progress with this issue?

The ticketing system does not support encryption but they answered
positively, I received a key and the ball's in my court now.  There was
less room in my schedule than I expected, but I'll finally get back to
you these days with the contribution, this time for real.

--
Phil Estival




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Tue, 13 May 2025 14:05:02 GMT) Full text and rfc822 format available.

Message #52 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Phil Estival <pe <at> 7d.nz>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: 76025 <at> debbugs.gnu.org, Michael Mauger <mmauger <at> protonmail.com>
Subject: Re: [PATCH] ob-sql: session + sql.el w/o prompt
Date: Tue, 13 May 2025 16:03:49 +0200
[Message part 1 (text/plain, inline)]
* [2025-02-15 16:25 +0000] Ihor Radchenko <yantar92 <at> posteo.net>:
> Ihor Radchenko <yantar92 <at> posteo.net> writes:
>
> May you please update your latest patch for ob-sql.el, converting it
> into (1) patch to sql.el; (2) patch for ob-sql.el that assumes changes
> to sql.el?

I'm submitting again the patch to sql.el with two additionnal modifications.
This replaces the previous one.

--
Phil Estival

[0001-lisp-progmodes-sql.el-login-without-prompting.patch (text/x-diff, attachment)]
[0002-lisp-progmodes-sql.el-sql-comint-sqlite-allows-nil-d.patch (text/x-diff, inline)]
From 18c84feeb1d3f368a7e0e90692efe7a6a5b7c64f Mon Sep 17 00:00:00 2001
From: Phil Estival <pe <at> 7d.nz>
Date: Tue, 13 May 2025 15:57:24 +0200
Subject: [PATCH 2/2] * lisp/progmodes/sql.el: sql-comint-sqlite allows nil
 database name

Connecting to SQLite with no database name provided (be it nil or empty
string), means to connect to the in-memory transient database.  Since no
parameter can be given, they are made optional.
---
 lisp/progmodes/sql.el | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/sql.el b/lisp/progmodes/sql.el
index af5b1a399cb..296f0336081 100644
--- a/lisp/progmodes/sql.el
+++ b/lisp/progmodes/sql.el
@@ -5075,13 +5075,15 @@ sql-sqlite
   (interactive "P")
   (sql-product-interactive 'sqlite buffer))
 
-(defun sql-comint-sqlite (product options &optional buf-name)
-  "Create comint buffer and connect to SQLite."
+(defun sql-comint-sqlite (product &optional options buf-name)
+  "Create a comint buffer and connect to SQLite (PRODUCT).
+See `sql-comint' for the OPTIONS and BUF-NAME parameters."
   ;; Put all parameters to the program (if defined) in a list and call
   ;; make-comint.
   (let ((params
          (append options
-                 (if (not (string= "" sql-database))
+                 (if (and sql-database
+                          (not (string= "" sql-database)))
                      `(,(expand-file-name sql-database))))))
     (sql-comint product params buf-name)))
 
-- 
2.39.5


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Wed, 14 May 2025 14:30:01 GMT) Full text and rfc822 format available.

Message #55 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Phil Estival <pe <at> 7d.nz>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: Michael Mauger <mmauger <at> protonmail.com>,
 Org Mode List <emacs-orgmode <at> gnu.org>, 76025 <at> debbugs.gnu.org
Subject: Re: [PATCH] ob-sql: session + sql.el w/o prompt
Date: Wed, 14 May 2025 16:29:34 +0200
[Message part 1 (text/plain, inline)]
* [2025-02-15 16:25 +0000] Ihor Radchenko <yantar92 <at> posteo.net>:
> Ihor Radchenko <yantar92 <at> posteo.net> writes:
>
> May you please update your latest patch for ob-sql.el, converting it
> into (1) patch to sql.el; (2) patch for ob-sql.el that assumes changes
> to sql.el?

Here they are
1) the patch of sql.el
2) the list of changes in ob-sql.el against release_9.7.30.
   The patch that assumes the change in sql.el is n°13.

Cheers
--
Phil Estival


[0001-lisp-progmodes-sql.el-login-without-prompting.patch (text/x-diff, attachment)]
[0002-lisp-progmodes-sql.el-sql-comint-sqlite-allows-nil-d.patch (text/x-diff, attachment)]
[0001-ob-sql-session-support.-Introduces-new-variables-and.patch (text/x-diff, attachment)]
[0002-ob-sql-default-header-arguments-are-declared-in-a-cu.patch (text/x-diff, attachment)]
[0003-ob-sql-remove-org-assert-version-to-stay-compatible-.patch (text/x-diff, attachment)]
[0004-ob-sql-realign-variables-for-improved-readability.patch (text/x-diff, attachment)]
[0005-ob-sql-fix-docstring.patch (text/x-diff, attachment)]
[0006-ob-sql-turn-a-unique-cond-expression-to-a-when-expre.patch (text/x-diff, attachment)]
[0007-ob-sql-new-function-for-session-and-db-connection-in.patch (text/x-diff, attachment)]
[0008-ob-sql-simplify-docstring.patch (text/x-diff, attachment)]
[0009-ob-sql-add-support-for-sessions-in-org-babel-execute.patch (text/x-diff, attachment)]
[0010-ob-sql-Fix-org-babel-expand-body-returning-extra-n.patch (text/x-diff, attachment)]
[0011-ob-sql-replace-call-to-intern-engine-by-the-previous.patch (text/x-diff, attachment)]
[0012-ob-sql-restore-support-for-sqlite.patch (text/x-diff, attachment)]
[0013-ob-sql-code-block-in-run-by-interacting-in-the-comin.patch (text/x-diff, attachment)]
[0014-ob-sql-update-commentary-reflecting-recent-changes-o.patch (text/x-diff, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Wed, 14 May 2025 16:11:01 GMT) Full text and rfc822 format available.

Message #58 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Phil Estival <pe <at> 7d.nz>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: Michael Mauger <mmauger <at> protonmail.com>,
 Org Mode List <emacs-orgmode <at> gnu.org>, 76025 <at> debbugs.gnu.org
Subject: Re: [PATCH] ob-sql: session + sql.el w/o prompt
Date: Wed, 14 May 2025 18:10:09 +0200
* [2025-05-14 16:29 +0200] Phil Estival <pe <at> 7d.nz>:
> * [2025-02-15 16:25 +0000] Ihor Radchenko <yantar92 <at> posteo.net>:
>> Ihor Radchenko <yantar92 <at> posteo.net> writes:
>>
>> May you please update your latest patch for ob-sql.el, converting it
>> into (1) patch to sql.el; (2) patch for ob-sql.el that assumes changes
>> to sql.el?
>
> Here they are
> 1) the patch of sql.el
> 2) the list of changes in ob-sql.el against release_9.7.30.
>    The patch that assumes the change in sql.el is n°13.

Erratum : it's already in n°7, where sql-product-interactive
is beeing called with 3 arguments.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Sat, 17 May 2025 18:07:01 GMT) Full text and rfc822 format available.

Message #61 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: Phil Estival <pe <at> 7d.nz>
Cc: Michael Mauger <mmauger <at> protonmail.com>,
 Org Mode List <emacs-orgmode <at> gnu.org>, 76025 <at> debbugs.gnu.org
Subject: Re: [PATCH] ob-sql: session + sql.el w/o prompt
Date: Sat, 17 May 2025 18:05:11 +0000
Phil Estival <pe <at> 7d.nz> writes:

>> May you please update your latest patch for ob-sql.el, converting it
>> into (1) patch to sql.el; (2) patch for ob-sql.el that assumes changes
>> to sql.el?
>
> Here they are
> 1) the patch of sql.el
> 2) the list of changes in ob-sql.el against release_9.7.30.
>    The patch that assumes the change in sql.el is n°13.

Thanks! I will comment on the Org-related part.

> Subject: [PATCH 01/14] ob-sql: session support. Introduces new variables and
>  functions
>
> * lisp/ob-sql.el: new variables and functions for session support for
> postgres and sqlite. Requires sql.el for a connection to a session.
> SQL clients are configured by a preamble of commands given to the SQL
> shell.  Custom variables are declared in a new sub-group ob-babel-sql.
> The echo of an SQL ANSI comment is to be appended to the source block
> of SQL commands for comint to detect when the commands terminate,
> instead of relying on prompt detection.

This is not a complete changelog of your changes. Please check out
https://orgmode.org/worg/org-contribute.html#commit-messages and follow
the format.

In general, because you plan to be the maintainer, please familiarize
yourself with all the conventions discussed in
https://orgmode.org/worg/org-contribute.html and
https://orgmode.org/worg/org-maintenance.html

>  ;; Author: Eric Schulte
>  ;; Maintainer: Daniel Kraus <daniel <at> kraus.my>
> +;; Maintainer: Philippe Estival <pe <at> 7d.nz>
>  ;; Keywords: literate programming, reproducible research
>  ;; URL: https://orgmode.org

I think adding you as a maintainer should be a separate patch to make
the change distinct.
  
>  (require 'ob)
> +(require 'sql)

Do we need to load sql.el early? May we do it dynamically instead?

> +(defvar org-babel-sql-session-start-time)

This variable is unused. What is it for?

> +(defvar org-sql-session-preamble
> +  (list
> +   'postgres "\\set ON_ERROR_STOP 1
> +\\pset footer off
> +\\pset pager off
> +\\pset format unaligned"	 )
> +  "Command preamble to run upon shell start.")

Should it be a defcustom?

> +(defvar org-sql-session-command-terminated nil)

AFAIU, this is some kind of flag to be used inside sql comit buffer.
Should it be buffer-local?
Also, please add a docstring.

> +(defvar org-sql-session--batch-terminate  "---#"  "To print at the end of a command batch.")

This is a bit weird formatting. In Elisp, the docstring is usually
placed on a separate line. Also, please check out
https://www.gnu.org/software/emacs/manual/html_node/elisp/Documentation-Tips.html
Your docstring here reads confusing.

> +(defvar org-sql-batch-terminate
> +  (list 'sqlite (format ".print %s\n" org-sql-session--batch-terminate)
> +        'postgres (format "\\echo %s\n" org-sql-session--batch-terminate))
> +  "Print the command batch termination as last command.")

This implies that you treat `org-sql-session--batch-terminate' as a
constant. If so, maybe declare it as such.

Also, should it be a defcustom?

> +(defvar org-sql-terminal-command-prefix
> +  (list 'sqlite "\\."
> +        'postgres "\\\\")
> +  "Identify a command for the SQL shell.")

This variable is unused.

> +(defvar org-sql-environment
> +  (list 'postgres '(("PGPASSWORD" sql-password))))

Also unused.

> +(defvar org-sql-session-clean-output nil
> +  "Store the regexp used to clear output (prompt1|termination|prompt2).")

Should it be buffer-local in session buffer?

> +(defvar org-sql-session-start-time)
> +(defvar org-sql-session-command-terminated nil)

Same question.

> +(defvar org-sql-session--batch-terminate  "---#"  "To print at the end of a command batch.")

Duplicate defvar.
  
> +(defcustom org-sql-run-comint-p 'nil

There is no need to quote nil and numbers.
They are self-quoting.

> +  "Run non-session SQL commands through comint if not nil."
> +  :type '(boolean)

:type 'boolean

> +(defcustom org-sql-timeout '5.0
> +  "Abort on timeout."
> +  :type '(number)

:type 'number

> +(defcustom org-sql-close-out-temp-buffer-p 'nil
> +  "To automatically close sql-out-temp buffer."

The docstring is not very clear.

> +  :type '(boolean)

:type 'boolean

> +(defun org-sql-session-comint-output-filter (_proc string)
> +  "Process output STRING of PROC gets redirected to a temporary buffer.
> +It is called several times consecutively as the shell outputs and flush
> +its message buffer"
> +
> +  ;; Inserting a result in the sql process buffer (to read it as a
> +  ;; regular prompt log) inserts it to the terminal, and as a result the
> +  ;; ouput would get passed as input onto the next command line; See
> +  ;; `comint-redirect-setup' to possibly fix that,
> +  ;; (with-current-buffer (process-buffer proc) (insert output))
> +
> +  (when (or (string-match org-sql-session--batch-terminate string)
> +            (> (time-to-seconds
> +                (time-subtract (current-time)
> +                               org-sql-session-start-time))
> +               org-sql-timeout))
> +    (setq org-sql-session-command-terminated t))
> +
> +  (with-current-buffer (get-buffer-create "*ob-sql-result*")
> +    (insert string)))

Using global `org-sql-session-command-terminated',
`org-sql-session-start-time', and "*ob-sql-result*" buffer will not be
reliable when there are multiple comint sessions. It will also make it
impossible to implement async sessions.
Please use some other approach (for example, buffer-local or
process-local variables)

> * lisp/ob-sql.el: removal of org-assert-version makes org-macs no
> longer needed.
> ...
> -
> -(require 'org-macs)
> -(org-assert-version)
> -

(org-assert-version) is needed to detect mixed Org versions.
(require 'org-macs) there is mostly to provide `org-assert-version'.

> Subject: [PATCH 04/14] ob-sql: realign variables for improved readability.

We do not allow whitespace-only commits. See
https://orgmode.org/worg/org-contribute.html#orgbbd6fd6

> +	  (when colnames-p (with-temp-buffer

nitpick: It is unusual to place
(when condition body-line1
               ...)

The common practice is
(when condition
  body)

> +        (setq org-sql-session-clean-output
> +              (plist-put org-sql-session-clean-output in-engine
> +                         (concat "\\(" prompt-regexp "\\)"
> +                                 "\\|\\(" org-sql-session--batch-terminate "\n\\)"
> +                                 (when prompt-cont-regexp
> +                                   (concat "\\|\\(" prompt-cont-regexp "\\)"))))))

This will be broken when multiple sessions for different engines are
used in parallel. Please avoid global state.

> +	     (let ((sql--buffer
> +		    (org-babel-sql-session-connect in-engine params session)))

Why `sql--bufer'? (double slash) It is not a variable sql.el uses.

> +	       (with-current-buffer (get-buffer-create "*ob-sql-result*")
> +		 (erase-buffer))
> +	       (setq org-sql-session-start-time (current-time))
> +	       (setq org-sql-session-command-terminated nil)
> +
> +	       (with-current-buffer (get-buffer sql--buffer)
> +		 (process-send-string (current-buffer)
> +				      (org-sql-session-format-query
> +                                       (org-babel-expand-body:sql body params)
> +                                       in-engine))
> +		 (while (or (not org-sql-session-command-terminated)
> +                            (> (time-to-seconds
> +                                (time-subtract (current-time)
> +                                               org-sql-session-start-time))
> +                               org-sql-timeout))
> +		   (sleep-for 0.03))
> +		 ;; command finished, remove filter
> +		 (set-process-filter (get-buffer-process sql--buffer) nil)

This looks repetitive with the code in
`org-sql-session-comint-output-filter'. Could it be possible to avoid
code duplication?

> +		 (when (not session-p)
> +		   (comint-quit-subjob)
> +		   ;; despite this quit signal, the process may not be finished yet
> +		   (let ((kill-buffer-query-functions nil))
> +		     (kill-this-buffer))))

Maybe we should wait until it is finished then? You can check `process-status'.

> Subject: [PATCH 11/14] ob-sql: replace call to (intern engine) by the 
>  previously declared in-engine
>
> TINYCHANGE

TINYCHANGE is unnecessary after you get the copyright assignment. (which
do you need to get)

> +                   (insert-for-yank

Why do you need `insert-for-yank'?

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Mon, 19 May 2025 01:37:04 GMT) Full text and rfc822 format available.

Message #64 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Michael Mauger <mmauger <at> protonmail.com>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: Phil Estival <pe <at> 7d.nz>, 76025 <at> debbugs.gnu.org,
 Org Mode List <emacs-orgmode <at> gnu.org>
Subject: Re: [PATCH] ob-sql: session + sql.el w/o prompt
Date: Mon, 19 May 2025 01:36:05 +0000
On Saturday, February 15th, 2025 at 4:25 PM, Ihor Radchenko <yantar92 <at> posteo.net> wrote:

> Ihor Radchenko yantar92 <at> posteo.net writes:
> 
> > May you please post the patch on Emacs bug tracker? (M-x
> > submit-emacs-patch) and X-Debbugs-CC me?
> > That way, we get the Emacs maintainers involved into the discussion.
> 
> 
> The patch is in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=76025
> 
> There are no major objections to the patch other than the maintainer
> being interested to coordinate ob-sql improvements and sql.el
> changes.
> 
> I am CCing the feature request thread so that we can discuss everything
> together.
> 
> May you please update your latest patch for ob-sql.el, converting it
> into (1) patch to sql.el; (2) patch for ob-sql.el that assumes changes
> to sql.el?
> 
> --
> Ihor Radchenko // yantar92,

I'm sorry to be late to this party, but this is the first real description of what features were needed from sql.el. 

Based on what I'm seeing, I think the feature you want is already present (but can be made simpler for you to use). And the support for the empty name sqlite database is similar to in-memory databases that I have in a local development branch that hasn't been merged/pushed.

The `sql-connect' machinery suppresses prompts for parameters in the connection alist already. There are two ways to take advantage of that: install connection parameters in `sql-connection-alist' with an ob-sql specific connection name and then `sql-connect' to it, or allow `sql-connect' to be called with a complete connection alist entry in place of the connection name. I think the latter option is a better choice, and easy to incorporate into the code; the former option would be a total ob-sql solution. Id be cautious modifying `sql-product-interactive' directly since so much passes thru there and `sql-connect' already implements the logic you want around it.

For sqlite, I've added support for an in-memory database using the name ":memory:" in my local repo. Obviously an empty name indicates a temporary database which may be in-memory, or if larger, in temporary disk space. I'll take a look at adding support for temporary databases but I'm not a huge fan of an empty database name option. Maybe support ":tempdb:" (or similar)?

I am looking to cleanup sql.el (some of it's code is 20+ years old) and has some very broken pieces in it. Changing it to simplify support from treesit and eglot exposes rough edges that will break things for existing users. `sql-connect' is the intended interface for programmatic invocation of SQL-interactive processes.

--
MICHAEL <at> MAUGER.COM // FSF and SFConservancy // GNU Emacs sql.el maintainer




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Sat, 24 May 2025 08:43:02 GMT) Full text and rfc822 format available.

Message #67 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Phil Estival <pe <at> 7d.nz>
Cc: 76025 <at> debbugs.gnu.org, yantar92 <at> posteo.net, michael <at> mauger.com
Subject: Re: bug#76025: [PATCH 1/1] * lisp/progmodes/sql.el: login without
 prompting
Date: Sat, 24 May 2025 11:42:34 +0300
> From: Phil Estival <pe <at> 7d.nz>
> Cc: yantar92 <at> posteo.net,  76025 <at> debbugs.gnu.org,  michael <at> mauger.com
> Date: Mon, 28 Apr 2025 17:52:54 +0200
> 
> * [2025-04-19 16:56 +0300] Eli Zaretskii <eliz <at> gnu.org>:
> >> From: Phil Estival <pe <at> 7d.nz>
> >> Cc: Ihor Radchenko <yantar92 <at> posteo.net>,  76025 <at> debbugs.gnu.org,
> >>   michael <at> mauger.com
> >> Date: Mon, 07 Apr 2025 13:59:47 +0200
> >>
> >> * [2025-03-18 22:48 +0200] Eli Zaretskii <eliz <at> gnu.org>:
> >> >> From: Ihor Radchenko <yantar92 <at> posteo.net>
> >> >> Cc: Eli Zaretskii <eliz <at> gnu.org>, 76025 <at> debbugs.gnu.org, michael <at> mauger.com
> >> >> Date: Tue, 18 Mar 2025 17:58:56 +0000
> >> >>
> >> >> Phil Estival <pe <at> 7d.nz> writes:
> >> >>
> >> >> >> From my POV, this change needs a NEWS entry, and I think Phil needs to
> >> >> >> do the legal paperwork of assigning the copyright to the FSF (I can
> >> >> >> send the form and the instructions if that's okay with Phil).
> >> >> >
> >> >> > I asked assign <at> gnu.org for a pgp pub key last Wednesday
> >> >> > but no response so far. Is there any?
> >> >>
> >> >> May I know if there is any progress?
> >> >
> >> > Not that I know of, no.
> >> >
> >> > I also didn't see any correspondence between Phil and the copyright
> >> > clerk, although usually the copyright clerk CC's me on his responses.
> >> >
> >> > Phil, did you send the form to the FSF?  Or are you still waiting for
> >> > their PGP key?
> >>
> >> I'm writing to them again to ask if there is one.
> >
> > Any progress with this issue?
> 
> The ticketing system does not support encryption but they answered
> positively, I received a key and the ball's in my court now.  There was
> less room in my schedule than I expected, but I'll finally get back to
> you these days with the contribution, this time for real.

Hi,

Any progress with this?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Sat, 07 Jun 2025 08:14:01 GMT) Full text and rfc822 format available.

Message #70 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: pe <at> 7d.nz
Cc: 76025 <at> debbugs.gnu.org, yantar92 <at> posteo.net, michael <at> mauger.com
Subject: Re: bug#76025: [PATCH 1/1] * lisp/progmodes/sql.el: login without
 prompting
Date: Sat, 07 Jun 2025 11:12:51 +0300
Ping!  Phil, any progress with the copyright assignment?

> Cc: 76025 <at> debbugs.gnu.org, yantar92 <at> posteo.net, michael <at> mauger.com
> Date: Sat, 24 May 2025 11:42:34 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> > From: Phil Estival <pe <at> 7d.nz>
> > Cc: yantar92 <at> posteo.net,  76025 <at> debbugs.gnu.org,  michael <at> mauger.com
> > Date: Mon, 28 Apr 2025 17:52:54 +0200
> > 
> > * [2025-04-19 16:56 +0300] Eli Zaretskii <eliz <at> gnu.org>:
> > >> From: Phil Estival <pe <at> 7d.nz>
> > >> Cc: Ihor Radchenko <yantar92 <at> posteo.net>,  76025 <at> debbugs.gnu.org,
> > >>   michael <at> mauger.com
> > >> Date: Mon, 07 Apr 2025 13:59:47 +0200
> > >>
> > >> * [2025-03-18 22:48 +0200] Eli Zaretskii <eliz <at> gnu.org>:
> > >> >> From: Ihor Radchenko <yantar92 <at> posteo.net>
> > >> >> Cc: Eli Zaretskii <eliz <at> gnu.org>, 76025 <at> debbugs.gnu.org, michael <at> mauger.com
> > >> >> Date: Tue, 18 Mar 2025 17:58:56 +0000
> > >> >>
> > >> >> Phil Estival <pe <at> 7d.nz> writes:
> > >> >>
> > >> >> >> From my POV, this change needs a NEWS entry, and I think Phil needs to
> > >> >> >> do the legal paperwork of assigning the copyright to the FSF (I can
> > >> >> >> send the form and the instructions if that's okay with Phil).
> > >> >> >
> > >> >> > I asked assign <at> gnu.org for a pgp pub key last Wednesday
> > >> >> > but no response so far. Is there any?
> > >> >>
> > >> >> May I know if there is any progress?
> > >> >
> > >> > Not that I know of, no.
> > >> >
> > >> > I also didn't see any correspondence between Phil and the copyright
> > >> > clerk, although usually the copyright clerk CC's me on his responses.
> > >> >
> > >> > Phil, did you send the form to the FSF?  Or are you still waiting for
> > >> > their PGP key?
> > >>
> > >> I'm writing to them again to ask if there is one.
> > >
> > > Any progress with this issue?
> > 
> > The ticketing system does not support encryption but they answered
> > positively, I received a key and the ball's in my court now.  There was
> > less room in my schedule than I expected, but I'll finally get back to
> > you these days with the contribution, this time for real.
> 
> Hi,
> 
> Any progress with this?
> 
> 
> 
> 




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Sun, 15 Jun 2025 15:17:02 GMT) Full text and rfc822 format available.

Message #73 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Phil Estival <pe <at> 7d.nz>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: Michael Mauger <mmauger <at> protonmail.com>,
 Org Mode List <emacs-orgmode <at> gnu.org>, 76025 <at> debbugs.gnu.org
Subject: Re: [PATCH] ob-sql: session
Date: Sun, 15 Jun 2025 17:16:08 +0200
[Message part 1 (text/plain, inline)]
I'm submitting again the patches for ob-sql.el, taking into
considerations the previous review, commented below. I also took good
note of Michael remarks.  The attached series of patches will work
without any modification to sql.el. I'll introduce later a local
`sql-connection' bound to the session name. In the absence of it,
sessions will work, but will ask to confirm provided connection
parameter upon establishing connection.

* [2025-05-17 18:05 +0000] Ihor Radchenko <yantar92 <at> posteo.net>:
> Phil Estival <pe <at> 7d.nz> writes:
>
>>> May you please update your latest patch for ob-sql.el, converting it
>>> into (1) patch to sql.el; (2) patch for ob-sql.el that assumes changes
>>> to sql.el?
>>
>> Here they are
>> 1) the patch of sql.el
>> 2) the list of changes in ob-sql.el against release_9.7.30.
>>    The patch that assumes the change in sql.el is n°13.
>
> Thanks! I will comment on the Org-related part.
>
>> Subject: [PATCH 01/14] ob-sql: session support. Introduces new variables and
>>  functions
>>
>> * lisp/ob-sql.el: new variables and functions for session support for
>> postgres and sqlite. Requires sql.el for a connection to a session.
>> SQL clients are configured by a preamble of commands given to the SQL
>> shell.  Custom variables are declared in a new sub-group ob-babel-sql.
>> The echo of an SQL ANSI comment is to be appended to the source block
>> of SQL commands for comint to detect when the commands terminate,l
>> instead of relying on prompt detection.
>
> This is not a complete changelog of your changes. Please check out
> https://orgmode.org/worg/org-contribute.html#commit-messages and follow
> the format.
>
> In general, because you plan to be the maintainer, please familiarize
> yourself with all the conventions discussed in
> https://orgmode.org/worg/org-contribute.html and
> https://orgmode.org/worg/org-maintenance.html
>
>>  ;; Author: Eric Schulte
>>  ;; Maintainer: Daniel Kraus <daniel <at> kraus.my>
>> +;; Maintainer: Philippe Estival <pe <at> 7d.nz>
>>  ;; Keywords: literate programming, reproducible research
>>  ;; URL: https://orgmode.org
>
> I think adding you as a maintainer should be a separate patch to make
> the change distinct.

Patch follows

>
>>  (require 'ob)
>> +(require 'sql)
>
> Do we need to load sql.el early? May we do it dynamically instead?
>

I see that calling `org-require-package' when establishing a new
connection is more rational. Changed in the following patch.

>> +(defvar org-babel-sql-session-start-time)
>
> This variable is unused. What is it for?
>

Starting time of one session block execution, used to exit on timeout.
Fixed in the following patch.

What naming convention to opt for ?  org-babel-[lang]-var ?
or ob-[lang]-var ?

>> +(defvar org-sql-session-preamble
>> +  (list
>> +   'postgres "\\set ON_ERROR_STOP 1
>> +\\pset footer off
>> +\\pset pager off
>> +\\pset format unaligned"	 )
>> +  "Command preamble to run upon shell start.")
>
> Should it be a defcustom?
>

Yes. It is now in the next patch.

>> +(defvar org-sql-session-command-terminated nil)
>
> AFAIU, this is some kind of flag to be used inside sql comit buffer.
> Should it be buffer-local?

Yes, it is now too.

> Also, please add a docstring.
>
>> +(defvar org-sql-session--batch-terminate  "---#"  "To print at the end of a command batch.")
>
> This is a bit weird formatting. In Elisp, the docstring is usually
> placed on a separate line. Also, please check out
> https://www.gnu.org/software/emacs/manual/html_node/elisp/Documentation-Tips.html
> Your docstring here reads confusing.

I hope the formatting and style of the others docstrings is now fine.

>
>> +(defvar org-sql-batch-terminate
>> +  (list 'sqlite (format ".print %s\n" org-sql-session--batch-terminate)
>> +        'postgres (format "\\echo %s\n" org-sql-session--batch-terminate))
>> +  "Print the command batch termination as last command.")
>
> This implies that you treat `org-sql-session--batch-terminate' as a
> constant. If so, maybe declare it as such.
>
> Also, should it be a defcustom?
>
>> +(defvar org-sql-terminal-command-prefix
>> +  (list 'sqlite "\\."
>> +        'postgres "\\\\")
>> +  "Identify a command for the SQL shell.")
>
> This variable is unused.
>

Right, it is no longer in the present mileage.

>> +(defvar org-sql-environment
>> +  (list 'postgres '(("PGPASSWORD" sql-password))))
>
> Also unused.
>

removed.

>> +(defvar org-sql-session-clean-output nil
>> +  "Store the regexp used to clear output (prompt1|termination|prompt2).")
>
> Should it be buffer-local in session buffer?
>

Yes.

>> +(defvar org-sql-session-start-time)
>> +(defvar org-sql-session-command-terminated nil)
>
> Same question.
>
>> +(defvar org-sql-session--batch-terminate  "---#"  "To print at the end of a command batch.")
>
> Duplicate defvar.
>
>> +(defcustom org-sql-run-comint-p 'nil
>
> There is no need to quote nil and numbers.
> They are self-quoting.
>
>> +  "Run non-session SQL commands through comint if not nil."
>> +  :type '(boolean)
>
> :type 'boolean
>
>> +(defcustom org-sql-timeout '5.0
>> +  "Abort on timeout."
>> +  :type '(number)
>
> :type 'number
>
>> +(defcustom org-sql-close-out-temp-buffer-p 'nil
>> +  "To automatically close sql-out-temp buffer."
>
> The docstring is not very clear.
>

Changed to "When non-nil, close the temporary buffer holding output results."

>> +  :type '(boolean)
>
> :type 'boolean
>
>> +(defun org-sql-session-comint-output-filter (_proc string)
>> +  "Process output STRING of PROC gets redirected to a temporary buffer.
>> +It is called several times consecutively as the shell outputs and flush
>> +its message buffer"
>> +
>> +  ;; Inserting a result in the sql process buffer (to read it as a
>> +  ;; regular prompt log) inserts it to the terminal, and as a result the
>> +  ;; ouput would get passed as input onto the next command line; See
>> +  ;; `comint-redirect-setup' to possibly fix that,
>> +  ;; (with-current-buffer (process-buffer proc) (insert output))
>> +
>> +  (when (or (string-match org-sql-session--batch-terminate string)
>> +            (> (time-to-seconds
>> +                (time-subtract (current-time)
>> +                               org-sql-session-start-time))
>> +               org-sql-timeout))
>> +    (setq org-sql-session-command-terminated t))
>> +
>> +  (with-current-buffer (get-buffer-create "*ob-sql-result*")
>> +    (insert string)))
>
> Using global `org-sql-session-command-terminated',
> `org-sql-session-start-time', and "*ob-sql-result*" buffer will not be
> reliable when there are multiple comint sessions. It will also make it
> impossible to implement async sessions.
> Please use some other approach (for example, buffer-local or
> process-local variables)
>


>> * lisp/ob-sql.el: removal of org-assert-version makes org-macs no
>> longer needed.
>> ...
>> -
>> -(require 'org-macs)
>> -(org-assert-version)
>> -
>
> (org-assert-version) is needed to detect mixed Org versions.
> (require 'org-macs) there is mostly to provide `org-assert-version'.
>

I reintroduced `org-macs to provide `org-require-package in the following
patches.  But shouldn't `org-assert-version be checked only once when
org core loads instead of subsequent modules?

>> Subject: [PATCH 04/14] ob-sql: realign variables for improved readability.
>
> We do not allow whitespace-only commits. See
> https://orgmode.org/worg/org-contribute.html#orgbbd6fd6
>

>> +	  (when colnames-p (with-temp-buffer
>
> nitpick: It is unusual to place
> (when condition body-line1
>                ...)
>
> The common practice is
> (when condition
>   body)
>

Ok, fixed.

>> +        (setq org-sql-session-clean-output
>> +              (plist-put org-sql-session-clean-output in-engine
>> +                         (concat "\\(" prompt-regexp "\\)"
>> +                                 "\\|\\(" org-sql-session--batch-terminate "\n\\)"
>> +                                 (when prompt-cont-regexp
>> +                                   (concat "\\|\\(" prompt-cont-regexp "\\)"))))))
>
> This will be broken when multiple sessions for different engines are
> used in parallel. Please avoid global state.
>

Set as local now.

>> +	     (let ((sql--buffer
>> +		    (org-babel-sql-session-connect in-engine params session)))
>
> Why `sql--bufer'? (double slash) It is not a variable sql.el uses.
>

Replaced by `ob-sql-buffer'.
IIUC, a double dash indicates a variable used by the prefixed package?

>> +	       (with-current-buffer (get-buffer-create "*ob-sql-result*")
>> +		 (erase-buffer))
>> +	       (setq org-sql-session-start-time (current-time))
>> +	       (setq org-sql-session-command-terminated nil)
>> +
>> +	       (with-current-buffer (get-buffer sql--buffer)
>> +		 (process-send-string (current-buffer)
>> +				      (org-sql-session-format-query
>> +                                       (org-babel-expand-body:sql body params)
>> +                                       in-engine))
>> +		 (while (or (not org-sql-session-command-terminated)
>> +                            (> (time-to-seconds
>> +                                (time-subtract (current-time)
>> +                                               org-sql-session-start-time))
>> +                               org-sql-timeout))
>> +		   (sleep-for 0.03))
>> +		 ;; command finished, remove filter
>> +		 (set-process-filter (get-buffer-process sql--buffer) nil)
>
> This looks repetitive with the code in
> `org-sql-session-comint-output-filter'. Could it be possible to avoid
> code duplication?
>

True, the evaluation of the timeout was dup.

>> +		 (when (not session-p)
>> +		   (comint-quit-subjob)
>> +		   ;; despite this quit signal, the process may not be finished yet
>> +		   (let ((kill-buffer-query-functions nil))
>> +		     (kill-this-buffer))))
>
> Maybe we should wait until it is finished then? You can check
> `process-status'.

To be fixed next time

>> +                   (insert-for-yank
>
> Why do you need `insert-for-yank'?

No need. It's rather insert instead.

Cheers,

--
Phil


[0001-ob-sql-maintainer.patch (text/x-diff, attachment)]
[0002-ob-sql-vars-local.patch (text/x-diff, inline)]
From e27f8231c2f97e6f7f84b6acb793eec7060b8396 Mon Sep 17 00:00:00 2001
From: Phil Estival <pe <at> 7d.nz>
Date: Wed, 13 June 2025 17:00:00 +0200
Subject: [PATCH 02/08] ob-sql: add session support

lisp/ob-sql.el: variables for session support to ob-sql

* ob-sql.el: declare variables and local variables
 for termination of a command batch,
 starting time of command execution for timeout,
 regexp to clear returned output

---

diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el
index 14ca6bc48..caee4502a 100644
--- a/lisp/ob-sql.el
+++ b/lisp/ob-sql.el
@@ -76,6 +77,23 @@

 (require 'ob)

+(defvar org-sql-session--batch-terminate  "---#"
+  "String echoed by the SQL at the end of a command batch.")
+
+(defvar org-sql-batch-terminate
+  (list 'sqlite (format ".print %s\n" org-sql-session--batch-terminate)
+        'postgres (format "\\echo %s\n" org-sql-session--batch-terminate))
+  "Print the command batch termination as last command.")
+
+(defvar-local org-sql-session-command-terminated nil
+  "Non-nil when a command batch has completed.")
+
+(defvar-local org-sql-session-start-time nil
+  "Starting time of a session code block execution, used to exit on timeout.")
+
+(defvar-local org-sql-session-clean-output nil
+  "Store the regexp used to clear output (prompt1|termination|prompt2).")
+
 (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))
[0003-ob-sql-custom-vars.patch (text/x-diff, inline)]
From e27f8231c2f97e6f7f84b6acb793eec7060b8396 Mon Sep 17 00:00:00 2001
From: Phil Estival <pe <at> 7d.nz>
Date: Wed, 13 June 2025 17:00:00 +0200
Subject: [PATCH 03/08] ob-sql: add session support

lisp/ob-sql.el: custom variables for session support to ob-sql

* ob-sql.el: declare custom variables
 default header argument per SQL engine,
 if non-session commands should also be run through comint,
 timeout duration of commands in seconds,
 if the temporary buffers of results are closed automatically

---

diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el
index 14ca6bc48..caee4502a 100644
--- a/lisp/ob-sql.el
+++ b/lisp/ob-sql.el

@@ -83,7 +99,49 @@
 (declare-function sql-set-product "sql" (product))

 (defvar sql-connection-alist)
-(defvar org-babel-default-header-args:sql '())
+
+(defcustom org-babel-default-header-args:sql  '((:engine . "unset"))
+  "Default header args."
+  :type '(alist :key-type symbol :value-type string
+                :options ("dbi" "sqlite" "mysql" "postgres"
+                          "sqsh" "mssql" "vertica" "oracle" "saphana" ))
+  :group 'org-babel-sql
+  :safe t)
+
+(defcustom org-sql-session-preamble
+  (list
+   'postgres "
+\\set ON_ERROR_STOP 1
+\\pset footer off
+\\pset pager off
+\\pset format unaligned"
+   'sqlite "
+.header off")
+  "Command preamble to run upon shell start."
+
+  :type '(plist :key-type symbol :value-type string
+                :options ('postgres 'sqlite 'dbi 'mysql
+                          'sqsh 'mssql 'vertica 'oracle 'saphana ))
+  :group 'org-babel-sql
+  :safe t)
+
+(defcustom org-sql-run-comint-p nil
+  "Run non-session SQL commands through comint if not nil."
+  :type 'boolean
+  :group 'org-babel-sql
+  :safe t)
+
+(defcustom org-sql-timeout 5.0
+  "Abort on timeout."
+  :type 'number
+  :group 'org-babel-sql
+  :safe t)
+
+(defcustom org-sql-close-out-temp-buffer-p nil
+  "When non-nil, close the temporary buffer holding output results."
+  :type 'boolean
+  :group 'org-babel-sql
+  :safe t)

 (defconst org-babel-header-args:sql
   '((engine	       . :any)
[0004-ob-sql-fix-prologue-epilogue.patch (text/x-diff, inline)]
From e27f8231c2f97e6f7f84b6acb793eec7060b8396 Mon Sep 17 00:00:00 2001
From: Phil Estival <pe <at> 7d.nz>
Date: Wed, 13 June 2025 17:00:00 +0200
Subject: [PATCH 04/08] ob-sql: clear nil values from prologue and epilogue

---

diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el
index 14ca6bc48..caee4502a 100644
--- a/lisp/ob-sql.el
+++ b/lisp/ob-sql.el


@@ -101,11 +159,11 @@
   (let ((prologue (cdr (assq :prologue params)))
 	(epilogue (cdr (assq :epilogue params))))
     (mapconcat 'identity
-               (list
+     (delq nil (list
                 prologue
                 (org-babel-sql-expand-vars
                  body (org-babel--get-vars params))
-                epilogue)
+                epilogue))
                "\n")))

 (defun org-babel-edit-prep:sql (info)
[0005-ob-sql-factorize-shell-quoting.patch (text/x-diff, inline)]
From e27f8231c2f97e6f7f84b6acb793eec7060b8396 Mon Sep 17 00:00:00 2001
From: Phil Estival <pe <at> 7d.nz>
Date: Wed, 13 June 2025 17:00:00 +0200
Subject: [PATCH 05/08] ob-sql: add session support

lisp/ob-sql.el: factorize shell quoting of connection parameters to SQL clients

@@ -117,11 +175,10 @@ corresponding :engine source block header argument."

 (defun org-babel-sql-dbstring-mysql (host port user password database)
   "Make MySQL cmd line args for database connection.  Pass nil to omit that arg."
-  (mapconcat
-   #'identity
+  (combine-and-quote-strings
    (delq nil
-	 (list (when host     (concat "-h" (shell-quote-argument host)))
+	 (list (when host     (concat "-h" host))
 	       (when port     (format "-P%d" port))
-	       (when user     (concat "-u" (shell-quote-argument user)))
-	       (when password (concat "-p" (shell-quote-argument password)))
-	       (when database (concat "-D" (shell-quote-argument database))))) " "))
+	       (when user     (concat "-u" user))
+	       (when password (concat "-p" password))
+	       (when database (concat "-D" database))))))

 (defun org-babel-sql-dbstring-postgresql (host port user database)
   "Make PostgreSQL command line args for database connection.
 Pass nil to omit that arg."
-  (mapconcat
-   #'identity
+  (combine-and-quote-strings
    (delq nil
-	 (list (when host (concat "-h" (shell-quote-argument host)))
+	 (list (when host (concat "-h" host))
 	       (when port (format "-p%d" port))
-	       (when user (concat "-U" (shell-quote-argument user)))
-	       (when database (concat "-d" (shell-quote-argument database))))) " "))
+	       (when user (concat "-U" user))
+	       (when database (concat "-d" database))))))

 (defun org-babel-sql-dbstring-oracle (host port user password database)
   "Make Oracle command line arguments for database connection.
[0006-ob-sql-connect-and-comint-filter.patch (text/x-diff, inline)]
From e27f8231c2f97e6f7f84b6acb793eec7060b8396 Mon Sep 17 00:00:00 2001
From: Phil Estival <pe <at> 7d.nz>
Date: Wed, 13 June 2025 17:00:00 +0200
Subject: [PATCH 06/08] ob-sql: add session support

lisp/ob-sql.el: preparation of session support to ob-sql

* ob-sql.el: add `org-babel-sql-session-connect' and `org-sql-session-comint-output-filter'

@@ -418,6 +508,79 @@ argument mechanism."

-(defun org-babel-prep-session:sql (_session _params)
-  "Raise an error because Sql sessions aren't implemented."
-  (error "SQL sessions not yet implemented"))
+(defun org-babel-sql-session-connect (in-engine params session)
+  "Start the SQL client of IN-ENGINE if it has not.
+PARAMS provides the sql connection parameters for a new or
+existing SESSION.  Clear the intermediate buffer from previous
+output, and set the process filter.  Return the comint process
+buffer."
+  (let* ((buffer-name (format "%s" (if (string= session "none") ""
+                                     (format "[%s]" session))))
+         (ob-sql-buffer (format "*SQL: %s*" buffer-name)))
+    (org-require-package 'sql)
+    ;; initiate a new connection
+    (when (not (org-babel-comint-buffer-livep ob-sql-buffer))
+      ;; store the regexp used to clear output (prompt1|indicator|prompt2)
+      (let ((prompt-regexp (sql-get-product-feature in-engine :prompt-regexp ))
+            (prompt-cont-regexp (sql-get-product-feature in-engine :prompt-cont-regexp)))
+        (setq-local org-sql-session-clean-output
+              (plist-put org-sql-session-clean-output in-engine
+                         (concat "\\(" prompt-regexp "\\)"
+                                 "\\|\\(" org-sql-session--batch-terminate "\n\\)"
+                                 (when prompt-cont-regexp
+                                   (concat "\\|\\(" prompt-cont-regexp "\\)"))))))
+
+      (let ((sql-server   (cdr (assoc :dbhost params)))
+            (port      (cdr (assoc :port params)))
+            (sql-database (cdr (assoc :database params)))
+            (sql-user     (cdr (assoc :dbuser params)))
+            (sql-password (cdr (assoc :dbpassword params))))
+        (when port (setq-local sql-port port))
+        (save-window-excursion
+          ;; provides environment expressions to the comint service
+          (let ((process-environment (copy-sequence process-environment))
+                (variables (sql-get-product-feature in-engine :sql-environment)))
+            (mapc (lambda (elem) ; evaluate environment expressions
+                    (setenv (car elem) (eval (cadr elem))))
+                  variables)
+            (sql-product-interactive in-engine buffer-name t)))
+
+        (let ((sql-term-proc (get-buffer-process ob-sql-buffer)))
+          (unless sql-term-proc
+            (user-error (format "SQL %s didn't start" in-engine)))
+
+          (with-current-buffer (get-buffer ob-sql-buffer)
+            ;; preamble commands
+            (let ((preamble (plist-get org-sql-session-preamble in-engine)))
+              (when preamble
+                (process-send-string ob-sql-buffer preamble)
+                (comint-send-input))))
+          ;; let the preamble execution finish and be filtered
+          (sleep-for 0.1))))
+
+    ;; set the redirection filter and return the SQL client buffer
+    (set-process-filter (get-buffer-process ob-sql-buffer)
+                        #'org-sql-session-comint-output-filter)
+    (get-buffer ob-sql-buffer)))
+
+(defun org-sql-session-comint-output-filter (_proc string)
+  "Process output STRING of PROC gets redirected to a temporary buffer.
+It is called several times consecutively as the shell outputs and flush
+its message buffer"
+
+  ;; Inserting a result in the sql process buffer (to read it as a
+  ;; regular prompt log) inserts it to the terminal, and as a result the
+  ;; ouput would get passed as input onto the next command line; See
+  ;; `comint-redirect-setup' to possibly fix that,
+  ;; (with-current-buffer (process-buffer proc) (insert output))
+
+  (when (or (string-match org-sql-session--batch-terminate string)
+            (> (time-to-seconds
+                (time-subtract (current-time)
+                               org-sql-session-start-time))
+               org-sql-timeout))
+    (setq-local org-sql-session-command-terminated t))
+
+  (with-current-buffer (get-buffer-create "*ob-sql-result*")
+    (insert string)))
+

 (provide 'ob-sql)
[0007-ob-sql-session-exec.patch (text/x-diff, inline)]
From e27f8231c2f97e6f7f84b6acb793eec7060b8396 Mon Sep 17 00:00:00 2001
From: Phil Estival <pe <at> 7d.nz>
Date: Wed, 13 June 2025 17:00:00 +0200
Subject: [PATCH 07/08] ob-sql: add session support

lisp/ob-sql.el: add execution of session support to ob-sql

* ob-sql.el: connect to an SQL shell and execute command block.
Replaces calls to (intern engine) by variable `in-engine',
Replace a unique cond by a when
Close temporary buffer when org-sql-close-out-temp-buffer-p is non-nil.

@@ -245,28 +293,71 @@ database connections."
                          (cdr (assoc-string dbconnection sql-connection-alist t))))))))

 (defun org-babel-execute:sql (body params)
-  "Execute a block of Sql code with Babel.
+  "Execute SQL BODY with PARAMS.
 This function is called by `org-babel-execute-src-block'."
   (let* ((result-params (cdr (assq :result-params params)))
          (cmdline (cdr (assq :cmdline params)))
-         (dbhost (org-babel-find-db-connection-param params :dbhost))
-         (dbport (org-babel-find-db-connection-param params :dbport))
-         (dbuser (org-babel-find-db-connection-param params :dbuser))
+         (dbhost     (org-babel-find-db-connection-param params :dbhost))
+         (dbport     (org-babel-find-db-connection-param params :dbport))
+         (dbuser     (org-babel-find-db-connection-param params :dbuser))
          (dbpassword (org-babel-find-db-connection-param params :dbpassword))
          (dbinstance (org-babel-find-db-connection-param params :dbinstance))
-         (database (org-babel-find-db-connection-param params :database))
+         (database   (org-babel-find-db-connection-param params :database))
          (engine (cdr (assq :engine params)))
+         (in-engine  (intern (or engine (user-error "Missing :engine"))))
          (colnames-p (not (equal "no" (cdr (assq :colnames params)))))
          (in-file (org-babel-temp-file "sql-in-"))
          (out-file (or (cdr (assq :out-file params))
                        (org-babel-temp-file "sql-out-")))
 	 (header-delim "")
-         (command (cl-case (intern engine)
+         (session (cdr (assoc :session params)))
+         (session-p (not (string= session "none"))))
+
+	 (if (or session-p org-sql-run-comint-p) ; run through comint
+	     (let* ((ob-sql-buffer (org-babel-sql-session-connect in-engine params session))
+                    (proc (get-buffer-process ob-sql-buffer)))
+	       (setq-local org-sql-session-start-time (current-time))
+	       (setq-local org-sql-session-command-terminated nil)
+	       (with-current-buffer (get-buffer ob-sql-buffer)
+                 (progn
+                   (goto-char (point-max))
+                   (insert
+                    (format "%s\n;\n%s"
+                            (org-babel-expand-body:sql
+                             (replace-regexp-in-string "[\t]" "" body) params)
+                            (plist-get org-sql-batch-terminate in-engine)))
+                   (comint-send-input nil t))
+		 (while (not org-sql-session-command-terminated)
+		   (sleep-for 0.03))
+		 ;; command finished, remove filter
+		 (set-process-filter proc nil)
+
+		 (when (not session-p)
+		   (comint-quit-subjob)
+                   (while (not (eq 'exit (process-status proc)))
+                     (sleep-for 0.1))
+		   ;; despite this quit signal, the process may not be finished yet
+		   (let ((kill-buffer-query-functions nil))
+		     (kill-this-buffer))))
+
+	       (with-current-buffer (get-buffer-create "*ob-sql-result*")
+		 (goto-char (point-min))
+		 ;; clear the output or prompt and termination
+		 (let ((clean-output (plist-get org-sql-session-clean-output in-engine)))
+		   (while (re-search-forward clean-output nil t)
+		     (replace-match "")))
+		 (write-file out-file)))
+
+	   (let ( ; else run one command line
+         (command (cl-case in-engine
                     (dbi (format "dbish --batch %s < %s | sed '%s' > %s"
 				 (or cmdline "")
 				 (org-babel-process-file-name in-file)
 				 "/^+/d;s/^|//;s/(NULL)/ /g;$d"
 				 (org-babel-process-file-name out-file)))
+                    (sqlite (format "sqlite3 < %s > %s"
+                                   (org-babel-process-file-name in-file)
+                                   (org-babel-process-file-name out-file)))
                     (monetdb (format "mclient -f tab %s < %s > %s"
 				     (or cmdline "")
 				     (org-babel-process-file-name in-file)

@@ -332,7 +421,7 @@ footer=off -F \"\t\"  %s -f %s -o %s %s"
                     (t (user-error "No support for the %s SQL engine" engine)))))
     (with-temp-file in-file
       (insert
-       (pcase (intern engine)
+       (pcase in-engine
 	 (`dbi "/format partbox\n")
          (`oracle "SET PAGESIZE 50000
 SET NEWPAGE 0

@@ -356,23 +445,22 @@ SET COLSEP '|'
        (org-babel-expand-body:sql body params)
        ;; "sqsh" requires "go" inserted at EOF.
        (if (string= engine "sqsh") "\ngo" "")))
-    (org-babel-eval command "")
+    (org-babel-eval command "")))
     (org-babel-result-cond result-params
       (with-temp-buffer
 	(progn (insert-file-contents-literally out-file) (buffer-string)))
       (with-temp-buffer
 	(cond
-	 ((memq (intern engine) '(dbi mysql postgresql postgres saphana sqsh vertica))
+	 ((memq in-engine '(dbi mysql postgresql postgres saphana sqsh vertica))
 	  ;; Add header row delimiter after column-names header in first line
-	  (cond
-	   (colnames-p
-	    (with-temp-buffer
+	  (when colnames-p
+            (with-temp-buffer
 	      (insert-file-contents out-file)
 	      (goto-char (point-min))
 	      (forward-line 1)
 	      (insert "-\n")
 	      (setq header-delim "-")
-	      (write-file out-file)))))
+	      (write-file out-file))))
 	 (t
 	  ;; Need to figure out the delimiter for the header row
 	  (with-temp-buffer
@@ -388,6 +476,8 @@ SET COLSEP '|'
 	      (forward-char -1))
 	    (write-file out-file))))
 	(org-table-import out-file (if (string= engine "sqsh") '(4) '(16)))
+	(when org-sql-close-out-temp-buffer-p
+	  (kill-buffer (get-file-buffer out-file)))
 	(org-babel-reassemble-table
 	 (mapcar (lambda (x)
 		   (if (string= (car x) header-delim)
[0008-ob-sql-session-commentary.patch (text/x-diff, inline)]
From e27f8231c2f97e6f7f84b6acb793eec7060b8396 Mon Sep 17 00:00:00 2001
From: Phil Estival <pe <at> 7d.nz>
Date: Wed, 13 June 2025 17:00:00 +0200
Subject: [PATCH 08/08] ob-sql: add session support

lisp/ob-sql.el: Commentary of session support to ob-sql

@@ -36,6 +37,7 @@
 ;; Header args used:
 ;; - engine
 ;; - cmdline
+;; - sessions
 ;; - dbhost
 ;; - dbport
 ;; - dbuser
@@ -58,13 +60,20 @@
 ;; - mssql
 ;; - sqsh
 ;; - postgresql (postgres)
+;; - sqlite
 ;; - oracle
 ;; - vertica
 ;; - saphana
 ;;
+;; Limitation:
+;; - sessions:
+;;   - engines configured: sqlite, postgres
+;;   - no error line number (stays as LINE 1)
+;;   - default port number only
+;;
 ;; TODO:
 ;;
-;; - support for sessions
 ;; - support for more engines
-;; - what's a reasonable way to drop table data into SQL?
-;;
+;; - babel tables as input
+;; - raw replace result
+;; - port number configuration for sessions

 ;;; Code:

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Sun, 15 Jun 2025 21:00:02 GMT) Full text and rfc822 format available.

Message #76 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Michael Mauger <mmauger <at> protonmail.com>
To: Phil Estival <pe <at> 7d.nz>
Cc: 76025 <at> debbugs.gnu.org, Ihor Radchenko <yantar92 <at> posteo.net>,
 Org Mode List <emacs-orgmode <at> gnu.org>
Subject: Re: [PATCH] ob-sql: session
Date: Sun, 15 Jun 2025 20:59:13 +0000
On Sunday, June 15th, 2025 at 3:16 PM, Phil Estival <pe <at> 7d.nz> wrote:

> I also took good note of Michael remarks. 
> The attached series of patches will work
> without any modification to sql.el. I'll introduce later a local
> `sql-connection' bound to the session name. In the absence of it,
> sessions will work, but will ask to confirm provided connection
> parameter upon establishing connection.
> 

Wouldn't the SQLi buffer created by sql-connect be the session? At least in sql.el, when it uses either sql-connect or sql-product-interactive, all the buffers are scanned to for the desired connection settings and switch to the matching buffer rather than creating a new buffer. 

I am trying to cleanup sql.el 2.x before I do a more significant rewrite for sql.el 3.0.  The update will include sql-add-connection which cleanly adds an entry to sql-connection-alist and the ability to call sql-connect with with the connection settings themselves rather than the connection name.  Hopefully those will simplify the integrations that you need. 

And please let me know if you are not getting the behavior you want, or if you are working around something that I ought to look at....

-- 
MICHAEL <at> MAUGER.COM // FSF and SFConservancy // GNU Emacs sql.el maintainer




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Mon, 16 Jun 2025 12:20:03 GMT) Full text and rfc822 format available.

Message #79 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Phil Estival <pe <at> 7d.nz>
To: Michael Mauger <mmauger <at> protonmail.com>
Cc: 76025 <at> debbugs.gnu.org, Ihor Radchenko <yantar92 <at> posteo.net>,
 Org Mode List <emacs-orgmode <at> gnu.org>
Subject: Re: [PATCH] ob-sql: session
Date: Mon, 16 Jun 2025 14:19:18 +0200
* [2025-06-15 20:59 +0000] Michael Mauger <mmauger <at> protonmail.com>:
> On Sunday, June 15th, 2025 at 3:16 PM, Phil Estival <pe <at> 7d.nz> wrote:
>
>> I also took good note of Michael's remarks.
>> The attached series of patches will work
>> without any modification to sql.el. I'll introduce later a local
>> `sql-connection' bound to the session name. In the absence of it,
>> sessions will work, but will ask to confirm provided connection
>> parameter upon establishing connection.
>>
>
> Wouldn't the SQLi buffer created by sql-connect be the session?

Yes, right.  The session is an identifier name, the buffer's name is
build on that name but the connection's name can be shared among
sessions.

> At least in sql.el, when it uses either sql-connect or
> sql-product-interactive, all the buffers are scanned to for the
> desired connection settings and switch to the matching buffer rather
> than creating a new buffer.

Aren't they opened in parallel, one connection each, with the same
connection parameters ?

`org-babel-sql-session-connect' uses `sql-product-interactive' with a
buffer name as argument. Different sessions are run in different sql
interactive buffers, yet they have the same connection settings.

> I am trying to cleanup sql.el 2.x before I do a more significant
> rewrite for sql.el 3.0.  The update will include sql-add-connection
> which cleanly adds an entry to sql-connection-alist and the ability to
> call sql-connect with with the connection settings themselves rather
> than the connection name.  Hopefully those will simplify the
> integrations that you need.

Fine.

> And please let me know if you are not getting the behavior you want,
> or if you are working around something that I ought to look at....

* [2025-05-19 01:36 +0000] Michael Mauger <mmauger <at> protonmail.com>:
> For sqlite, I've added support for an in-memory database using the
> name ":memory:" in my local repo. Obviously an empty name indicates a
> temporary database which may be in-memory, or if larger, in temporary
> disk space. I'll take a look at adding support for temporary databases
> but I'm not a huge fan of an empty database name option. Maybe support
> ":tempdb:" (or similar)?

The message sqlite3 displays on the welcoming message is:
"Connected to a transient in-memory database.", so perhaps
the proper word is ":in-memory:" ?

While the empty string as database name should IMHO be avoided, the nil
parameter must still be allowed:

"If no database name is supplied, the ATTACH sql command can be used to
attach to existing or create new database files.  ATTACH can also be
used to attach to multiple databases within the same interactive
session.  This is useful for migrating data between databases, possibly
changing the schema along the way."

Cheers,
--
Phil Estival




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Mon, 16 Jun 2025 12:27:05 GMT) Full text and rfc822 format available.

Message #82 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Phil Estival <pe <at> 7d.nz>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: Michael Mauger <mmauger <at> protonmail.com>,
 Org Mode List <emacs-orgmode <at> gnu.org>, 76025 <at> debbugs.gnu.org
Subject: Re: [PATCH] ob-sql: session + sql.el w/o prompt
Date: Mon, 16 Jun 2025 14:26:13 +0200
[Message part 1 (text/plain, inline)]
This replaces the previous patch.  The `sql-product-interactive' call
was still using the modified signature and does no longer.

[0006-ob-sql-connect-and-comint-filter.patch (text/x-diff, attachment)]
[Message part 3 (text/plain, inline)]
--
Phil Estival

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Wed, 18 Jun 2025 01:22:05 GMT) Full text and rfc822 format available.

Message #85 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Michael Mauger <mmauger <at> protonmail.com>
To: Phil Estival <pe <at> 7d.nz>
Cc: 76025 <at> debbugs.gnu.org, Ihor Radchenko <yantar92 <at> posteo.net>,
 Org Mode List <emacs-orgmode <at> gnu.org>
Subject: Re: [PATCH] ob-sql: session
Date: Wed, 18 Jun 2025 01:15:11 +0000
On Monday, June 16th, 2025 at 12:19 PM, Phil Estival <pe <at> 7d.nz> wrote:

> * [2025-06-15 20:59 +0000] Michael Mauger mmauger <at> protonmail.com:
> 
> > On Sunday, June 15th, 2025 at 3:16 PM, Phil Estival pe <at> 7d.nz wrote:
> > 
> > > I also took good note of Michael's remarks.
> > > The attached series of patches will work
> > > without any modification to sql.el. I'll introduce later a local
> > > `sql-connection' bound to the session name. In the absence of it,
> > > sessions will work, but will ask to confirm provided connection
> > > parameter upon establishing connection.
> > 
> > Wouldn't the SQLi buffer created by sql-connect be the session?
> 
> 
> Yes, right. The session is an identifier name, the buffer's name is
> build on that name but the connection's name can be shared among
> sessions.
> 

I'd suggest remembering the buffer itself rather than the buffer name. Also, like `sql-product-interactive', you can control a portion of the buffer name in the `sql-connect' call. But again, it would be safer to use the buffer itself to memoize the session. Your org session name would translate to the buffer rather than the potentially changed buffer name.

> > At least in sql.el, when it uses either sql-connect or
> > sql-product-interactive, all the buffers are scanned to for the
> > desired connection settings and switch to the matching buffer rather
> > than creating a new buffer.
> 
> 
> Aren't they opened in parallel, one connection each, with the same
> connection parameters ?
> 
> `org-babel-sql-session-connect' uses` sql-product-interactive' with a
> buffer name as argument. Different sessions are run in different sql
> interactive buffers, yet they have the same connection settings.
> 

The name will be used as part of the match, so using different names will become different buffers. But I need to look at that logic a bit to insure it has consistent behavior.

> > I am trying to cleanup sql.el 2.x before I do a more significant
> > rewrite for sql.el 3.0. The update will include sql-add-connection
> > which cleanly adds an entry to sql-connection-alist and the ability to
> > call sql-connect with with the connection settings themselves rather
> > than the connection name. Hopefully those will simplify the
> > integrations that you need.
> 
> 
> Fine.
> 
> > And please let me know if you are not getting the behavior you want,
> > or if you are working around something that I ought to look at....
> 
> 
> * [2025-05-19 01:36 +0000] Michael Mauger mmauger <at> protonmail.com:
> 
> > For sqlite, I've added support for an in-memory database using the
> > name ":memory:" in my local repo. Obviously an empty name indicates a
> > temporary database which may be in-memory, or if larger, in temporary
> > disk space. I'll take a look at adding support for temporary databases
> > but I'm not a huge fan of an empty database name option. Maybe support
> > ":tempdb:" (or similar)?
> 
> 
> The message sqlite3 displays on the welcoming message is:
> "Connected to a transient in-memory database.", so perhaps
> the proper word is ":in-memory:" ?
> 
> While the empty string as database name should IMHO be avoided, the nil
> parameter must still be allowed:
> 
> "If no database name is supplied, the ATTACH sql command can be used to
> attach to existing or create new database files. ATTACH can also be
> used to attach to multiple databases within the same interactive
> session. This is useful for migrating data between databases, possibly
> changing the schema along the way."
> 

The nil database should work already, but really means ":memory:" which is sqlite's default in-memory database, in which ATTACH commands can still be used. 

> Cheers,
> --
> Phil Estival

I have been dealing with sick family for the past month and will be unavailable for at least the next week. I expect to have some coherent changes to sql.el by early July; I apologize for the delay but its been a chaotic 6 weeks but appears to finally be settling down... 

-- 
MICHAEL <at> MAUGER.COM // FSF and SFConservancy // GNU Emacs sql.el maintainer




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Thu, 19 Jun 2025 13:16:02 GMT) Full text and rfc822 format available.

Message #88 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: Phil Estival <pe <at> 7d.nz>
Cc: Michael Mauger <mmauger <at> protonmail.com>,
 Org Mode List <emacs-orgmode <at> gnu.org>, 76025 <at> debbugs.gnu.org
Subject: Re: [PATCH] ob-sql: session
Date: Thu, 19 Jun 2025 13:13:38 +0000
Phil Estival <pe <at> 7d.nz> writes:

> I'm submitting again the patches for ob-sql.el, taking into
> considerations the previous review, commented below. I also took good
> note of Michael remarks.  The attached series of patches will work
> without any modification to sql.el. I'll introduce later a local
> `sql-connection' bound to the session name. In the absence of it,
> sessions will work, but will ask to confirm provided connection
> parameter upon establishing connection.

Thanks!

> What naming convention to opt for ?  org-babel-[lang]-var ?
> or ob-[lang]-var ?

org-babel-[lang]-var

> I reintroduced `org-macs to provide `org-require-package in the following
> patches.  But shouldn't `org-assert-version be checked only once when
> org core loads instead of subsequent modules?

The purpose of org-assert-version is recording Org version information
in the compiled .elc file (it is a macro expanded at compile time). That
information is later compared with the dynamically determined Org
version, warning about mismatch. This approach allows us catching a
common problem when parts of Org libraries are loaded from built-in Org
mode (inside Emacs installation) and other parts are loaded from
external Org version (e.g. from ELPA). So, it should be in each Org
library - to make sure that every Org library is from the same Org version.

>> Why `sql--bufer'? (double slash) It is not a variable sql.el uses.
>>
>
> Replaced by `ob-sql-buffer'.
> IIUC, a double dash indicates a variable used by the prefixed package?

The usual convention is
1. <package>-... is a variable that belongs to <package>
2. <package>--... is an internal variable that belongs to <package>

So, your `sql--buffer' can be read as a variable that belongs to sql.el
itself. We may even accidentally shadow sql.el if it ever decided to
introduce such variable.

>>> +		 (when (not session-p)
>>> +		   (comint-quit-subjob)
>>> +		   ;; despite this quit signal, the process may not be finished yet
>>> +		   (let ((kill-buffer-query-functions nil))
>>> +		     (kill-this-buffer))))
>>
>> Maybe we should wait until it is finished then? You can check
>> `process-status'.
>
> To be fixed next time

Ok. Leaving it in this message for easier review next time.

> From e27f8231c2f97e6f7f84b6acb793eec7060b8396 Mon Sep 17 00:00:00 2001
> From: Phil Estival <pe <at> 7d.nz>
> Date: Wed, 13 June 2025 17:00:00 +0200
> Subject: [PATCH 01/08] ob-sql: add session support
>
> lisp/ob-sql.el: add session support to ob-sql
>
> * ob-sql.el: declare the maintainer of the following changes

There is no notion of maintainer for specific changes.
Simply say: ob-sql: Add Philippe Estival as a new maintainer

Daniel is not active on the list since 2023, so we should probably
remove him. Will need to ping him later to confirm that he is really
AWOL.

> +(defvar org-sql-batch-terminate
> +  (list 'sqlite (format ".print %s\n" org-sql-session--batch-terminate)
> +        'postgres (format "\\echo %s\n" org-sql-session--batch-terminate))
> +  "Print the command batch termination as last command.")

"Print the command batch termination as last command." sounds like this
is a function that prints the command ...

Maybe
"Alist of SQL commands used to print end marker after a command batch.
Keys of the alist are symbols like `sqlite', `postgress', etc.
Values of the alist are the SQL commands."

> +(defvar-local org-sql-session-command-terminated nil
> +  "Non-nil when a command batch has completed.")

Maybe
"Non-nil when a command batch has completed in current SQL session buffer."

> +(defvar-local org-sql-session-start-time nil
> +  "Starting time of a session code block execution, used to exit on timeout.")

Similar here. I believe that specifying that the variable is about
session buffer will make things more clear.

> +(defvar-local org-sql-session-clean-output nil
> +  "Store the regexp used to clear output (prompt1|termination|prompt2).")

This also sounds like a function "store" (action).


Maybe
"Regexp to filter out from SQL command output.
This variable is set buffer-local in SQL session buffer to
filter out prompts and termination strings."

> +(defcustom org-babel-default-header-args:sql  '((:engine . "unset"))

I think "none" will be a bit more idiomatic for Org babel.

> +  "Default header args."
> +  :type '(alist :key-type symbol :value-type string
> +                :options ("dbi" "sqlite" "mysql" "postgres"
> +                          "sqsh" "mssql" "vertica" "oracle" "saphana" ))
> +  :group 'org-babel-sql
> +  :safe t)

I do not think that we should make this particular variable a defcustom.
Simply because none of other babel backends do it.

Also, the list of SQL engines may probably be stored in a constant, so
that we can reuse it.

> +(defcustom org-sql-session-preamble
> +  (list
> +   'postgres "
> +\\set ON_ERROR_STOP 1
> +\\pset footer off
> +\\pset pager off
> +\\pset format unaligned"
> +   'sqlite "
> +.header off")
> +  "Command preamble to run upon shell start."
> +
> +  :type '(plist :key-type symbol :value-type string
> +                :options ('postgres 'sqlite 'dbi 'mysql
> +                          'sqsh 'mssql 'vertica 'oracle 'saphana ))
> +  :group 'org-babel-sql
> +  :safe t)

I do not think that :safe t is appropriate for the code that will be
injected into every SQL block.

> +(defcustom org-sql-run-comint-p nil
> +  "Run non-session SQL commands through comint if not nil."

The idiomatic way is "If non-nil, ..."

> +  :type 'boolean
> +  :group 'org-babel-sql
> +  :safe t)
> +
> +(defcustom org-sql-timeout 5.0
> +  "Abort on timeout."

Action is always confusing at the beginning of variable docstring.
I suggest
"Timeout, in seconds, before ob-sql aborts code evaluation."

>  (defun org-babel-sql-dbstring-mysql (host port user password database)
>    "Make MySQL cmd line args for database connection.  Pass nil to omit that arg."
> -  (mapconcat
> -   #'identity
> +  (combine-and-quote-strings

`combine-and-quote-string' is _not_ the same as `shell-quote-argument'.
In fact, it has nothing to do with shell
quoting. `combine-and-quote-string' is about Elisp string quoting.
I recommend looking into the source code of these two functions. The
difference will be obvious.

> Subject: [PATCH 06/08] ob-sql: add session support

(I used the version of the patch you posted
https://list.orgmode.org/orgmode/87wm9b6ffe.fsf <at> 7d.nz/)

> +        (setq-local org-sql-session-clean-output
> +              (plist-put org-sql-session-clean-output in-engine
> +                         (concat "\\(" prompt-regexp "\\)"
> +                                 "\\|\\(" org-sql-session--batch-terminate "\n\\)"
> +                                 (when prompt-cont-regexp
> +                                   (concat "\\|\\(" prompt-cont-regexp "\\)"))))))

Since the variable is buffer-local, do we really need to store
per-engine values? Can be there multiple engines in the same SLQ comint
buffer?

> +(defun org-sql-session-comint-output-filter (_proc string)
> +  "Process output STRING of PROC gets redirected to a temporary buffer.
> +It is called several times consecutively as the shell outputs and flush
> +its message buffer"
> ...
> +
> +  (when (or (string-match org-sql-session--batch-terminate string)
> +            (> (time-to-seconds
> +                (time-subtract (current-time)
> +                               org-sql-session-start-time))
> +               org-sql-timeout))
> +    (setq-local org-sql-session-command-terminated t))

But that won't actually terminate the command when we are over
`org-sql-timeout', right? So, if something hangs, we will be unable to
send any new src blocks to this SQL buffer (with :session).

> +                    (format "%s\n;\n%s"
> +                            (org-babel-expand-body:sql
> +                             (replace-regexp-in-string "[\t]" "" body) params)

Why do you need an additional cleanup of the BODY? Can't it be done
inside `org-babel-expand-body:sql' itself?

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Thu, 19 Jun 2025 17:02:03 GMT) Full text and rfc822 format available.

Message #91 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: Michael Mauger <mmauger <at> protonmail.com>
Cc: Phil Estival <pe <at> 7d.nz>, 76025 <at> debbugs.gnu.org,
 Org Mode List <emacs-orgmode <at> gnu.org>
Subject: Re: [PATCH] ob-sql: session
Date: Thu, 19 Jun 2025 17:00:05 +0000
Michael Mauger <mmauger <at> protonmail.com> writes:

> I'd suggest remembering the buffer itself rather than the buffer name. Also, like `sql-product-interactive', you can control a portion of the buffer name in the `sql-connect' call. But again, it would be safer to use the buffer itself to memoize the session. Your org session name would translate to the buffer rather than the potentially changed buffer name.

Org mode markup communicates "session" by naming it.
So, one way or another, Org mode APIs expect some way to link session
name string to actual buffer.

API-wise, it is done via `org-babel-session-buffer', which can
optionally use special function org-babel-session-buffer:sql (if
defined) to perform backend-specific logic.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76025; Package emacs. (Thu, 19 Jun 2025 19:33:08 GMT) Full text and rfc822 format available.

Message #94 received at 76025 <at> debbugs.gnu.org (full text, mbox):

From: Michael Mauger <mmauger <at> protonmail.com>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: Phil Estival <pe <at> 7d.nz>, "76025 <at> debbugs.gnu.org" <76025 <at> debbugs.gnu.org>,
 Org Mode List <emacs-orgmode <at> gnu.org>
Subject: Re: [PATCH] ob-sql: session
Date: Thu, 19 Jun 2025 19:32:01 +0000
-------- Original Message --------
On 6/19/25 1:01 PM, Ihor Radchenko <yantar92 <at> posteo.net> wrote:

>  Michael Mauger <mmauger <at> protonmail.com> writes:
>  
>  > I'd suggest remembering the buffer itself rather than the buffer name. Also, like `sql-product-interactive', you can control a portion of the buffer name in the `sql-connect' call. But again, it would be safer to use the buffer itself to memoize the session. Your org session name would translate to the buffer rather than the potentially changed buffer name.
>  
>  Org mode markup communicates "session" by naming it.
>  So, one way or another, Org mode APIs expect some way to link session
>  name string to actual buffer.
>  
>  API-wise, it is done via `org-babel-session-buffer', which can
>  optionally use special function org-babel-session-buffer:sql (if
>  defined) to perform backend-specific logic.
>  

My suggestion was to store the stable buffer object with the session. The API can serve up the name of that's what is what is expected but recognize that while the buffer is stable, the buffer name might not be depending upon the users configuration. I also want to make sure that you are not relying upon some naming convention of SQLi buffer names to your session names. 

In the end, these are org decisions but I cannot guarantee stability of the buffer naming convention generated by sql-mode going forward…




This bug report was last modified today.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.