* [2025-01-07 19:38] Ihor Radchenko:> Phil Estival <pe@7d.nz> writes:
>
> [...] these `sql-set-product-feature' calls are overriding the defaults
> from sql.el. They will not only affect Org babel blocks, but all the
> interactive SQL sessions in Emacs. Such side effects are not acceptable.
> May we somehow avoid modifying pre-existing sql features?

We do. I replaced them with a plist.  The variables given on the
command line are now set by a preamble upon shell connection, at least
for Postgres.  However, some SQL shells, like Mariadb, can only be
configured through command line parameters. These are global in Emacs
and set by `sql-set-product-feature'. So in order to support these,
the function in charge of opening the connection needs different
parameters. I could get it to run to run by setting `sql-mysql-options' 
to "-s" , e.g. silent, but with an error on the prompt because
(sql-get-product-feature 'mysql :prompt-regexp) is "^mysql> ".
In conclusion, they're not exactly interchangeable even if one holds
several functions that are aliases from the other.

> It is probably OK to add Org-specific settings after prepending them
> with org-. For example, :batch-terminate -> :org-batch-terminate.
> Although, I am not sure what is the benefit of storing these _new_
> settings in the `sql-product-alist'.

Right.

>
>> +(defun ob-sql-session-buffer-live-p (buffer)
>
> May you simply use `org-babel-comint-buffer-livep' instead?

Indeed. And it keeps working when sql-interactive is activated in
this comint buffer.

>> +The buffer naming was shortened from
>> +*[session] engine://user@host/database*,
>> +that clearly identifies the connection from Emacs,
>> +to *SQL [session]* in order to retrieve a session with its
>> +name alone, the other parameters in the header args being
>> +no longer needed while the session stays open."

This comment was related to the need to store the connections
and in the future provide a way to quickly visualize
and manage them. Apparently sql.el also store and retrieve connections.

>> +  (sql-set-product in-engine)
>> ...
>
> Is there any specific reason why you are seemingly re-implementing what
> `sql-product-interactive' does? May we re-use it instead?
>

Yes. When opening a new connection `sql-product-interactive' will
systematically ask for input in the mini-buffer prompt and fill the
required information, suggesting inputs from the ones previously given.

The expected behavior is to silently establish the new connection
with the variables provided when the header arguments are complete,
and ask for a prompt on the mini-buffer only if a variable is missing
or if the client forcefully ask for it.
Also we rather not run `sql-login-hook' as they can have side effects
that would prevent the session to behave as expected.
I will check again if sql.el has a function to recall existing closed
connections without prompting for confirmation.

Please, find attached the updated patch for review.

voilà.

Phil