Re: [PATCH] ob-sql: session + sql.el w/o prompt

[Phil Estival <pe@7d.nz>]

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

* [2025-01-17 Fri 06:58 +0100] Ihor Radchenko <yantar92@posteo.net>
> Phil Estival <pe@7d.nz> writes:
> 
>>> 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.
> 
> What about changing sql.el to provide the necessary flexibility?
> I'd prefer it better than rewriting parts of sql.el in Org mode.

Gladly. Here is a proposal for patch for `sql-product-interactive' in
sql.el. A specific case for sqlite is handled and this is not 100%
satisfying. I have the feeling this should rather be where a
conversion of nil params to empty string happens, but sql.el is
rather long and the verifications required are numerous... There may
also be other db that allow connection without one these parameters...

The previous configuration from this series of patch, with a variant
of `sql-product-interactive', would allow a nil database for a session
of sqlite, now it needs to mention an empty string for :database or to
set it as a default value in `sql-sqlite-login-params'.

Next is the patch for ob-sql that rely on the modification of
`sql-product-interactive' to keep session connection smooth.  This
substract an addition from the previous series of patch but I guess
the proper way to do will be to submit the diff from main after
these reconsiderations.

A side note: there are contradictory comments in sql.el.
- `sql-product-interactive'
      Do not call this function by yourself.  The environment must be
      initialized by an entry function specific for the SQL interpreter.
      See `sql-help' for a list of available entry functions.
- `sql-help'
      You can also use M-x sql-product-interactive to invoke the
      interpreter for the current ‘sql-product’.
-- 
Phil Estival

[-- Attachment #2: 0001-lisp-progmodes-sql.el-login-without-prompting.patch --]
[-- Type: text/x-patch, Size: 2416 bytes --]

From 3fb0af62fd6154898f888f3a8a4d19162f7070bf Mon Sep 17 00:00:00 2001
From: Phil Estival <pe@7d.nz>
Date: Sun, 19 Jan 2025 01:57:37 +0100
Subject: [PATCH 1/1] * lisp/progmodes/sql.el: login without prompting

New optional parameter 'no-prompt' to `sql-product-interactive'.
When set with all connection parameters provided, skip the call to
`sql-get-login' to connect without opening the prompt, but
still prompt if any required parameter is missing.
Also skip prompt for sqlite when the database's name is en empty
string to connect to the in-memory transient database.
---
 lisp/progmodes/sql.el | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/sql.el b/lisp/progmodes/sql.el
index a1c50a06990..2b6303f1fe0 100644
--- a/lisp/progmodes/sql.el
+++ b/lisp/progmodes/sql.el
@@ -4524,9 +4524,10 @@ sql-connection-menu-filter

 ;;; Entry functions for different SQL interpreters.
 ;;;###autoload
-(defun sql-product-interactive (&optional product new-name)
+(defun sql-product-interactive (&optional product new-name no-prompt)
   "Run PRODUCT interpreter as an inferior process.

 If buffer `*SQL*' exists but no process is running, make a new process.
 If buffer exists and a process is running, just make sure buffer `*SQL*'
 is displayed.
+When no-prompt is set, try to connect without prompting.

 To specify the SQL product, prefix the call with
 \\[universal-argument].  To set the buffer name as well, prefix
@@ -4572,5 +4573,12 @@ sql-product-interactive
                   new-sqli-buffer rpt)

               ;; Get credentials.
-              (apply #'sql-get-login
-                     (sql-get-product-feature product :sqli-login))
+              (when (or (not no-prompt); default
+                        (and no-prompt ; verify if any parameter is missing
+                             (seq-some (lambda(x) (or (null x) (string-empty-p x)))
+                                       (list sql-database sql-user sql-server))
+                             ;; sqlite allows in-memory db, w/o login
+                             (not (and (eq product 'sqlite)
+                                       (equal "" sql-database)))))
+                (apply #'sql-get-login
+                       (sql-get-product-feature product :sqli-login)))

               ;; Connect to database.
               (setq rpt (sql-make-progress-reporter nil "Login"))
--
2.39.5

[-- Attachment #3: 0027-ob-sql-connect-a-session-with-sql-product-interactiv.patch --]
[-- Type: text/x-patch, Size: 7296 bytes --]

From b23f21866ebd485ba8a3df536131deb4f6fe8f73 Mon Sep 17 00:00:00 2001
From: Phil Estival <pe@7d.nz>
Date: Sun, 19 Jan 2025 09:32:29 +0100
Subject: [PATCH 27/27] ob-sql: connect a session with `sql-product-interactive'

* lisp/ob-sql.el: removes the variant of `sql-product-interactive'
and connects with `sql-product-interactive' patched with 'no-prompt'
in signature.  Provides environment variables to SQL on comint
Provides environment variables to comit specific for each SQL client
as a list of cons whose car is the name of the variable and the cdr
an expression. These variables are stored with
(sql-set-product-feature product :sql-environment).
---
 lisp/ob-sql.el | 122 ++++++++++++-------------------------------------
 1 file changed, 28 insertions(+), 94 deletions(-)

diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el
index 83d292ac8..845c59c60 100644
--- a/lisp/ob-sql.el
+++ b/lisp/ob-sql.el
@@ -553,18 +553,31 @@ buffer."
                                  "\\|\\(" org-sql-session--batch-terminate "\n\\)"
                                  (when prompt-cont-regexp
                                    (concat "\\|\\(" prompt-cont-regexp "\\)"))))))
-      (save-window-excursion
-        (setq ob-sql-buffer  ; start the client
-              (org-babel-sql-connect in-engine buffer-name params)))
-      (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)))
+
+      (let ((sql-server   (cdr (assoc :dbhost params)))
+	    ;; (sql-port      (cdr (assoc :port params))) ; todo
+	    (sql-database (cdr (assoc :database params)))
+	    (sql-user     (cdr (assoc :dbuser params)))
+	    (sql-password (cdr (assoc :dbpassword params))))
+
+	 ;; 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)
+          (save-window-excursion
+	     (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-babel-sql-connect (&optional engine sql-cnx params)
-  "Run ENGINE interpreter as an inferior process.
-SQL-CNX is the client buffer.  This is a variant from sql.el that prompt
-parametrs for authentication only if there's a missing parameter.
-Depending on the sql client the password should also be prompted."
-
-  (setq sql-product(cond
-                    ((assoc engine sql-product-alist) ; Product specified
-                     engine)
-                    (t sql-product))) ; or default to sql-engine
-
-  (when (sql-get-product-feature sql-product :sqli-comint-func)
-    (let (;(buf (sql-find-sqli-buffer sql-product sql-connection)) ; unused yet
-          (sql-server    (cdr (assoc :dbhost params)))
-          ;; (sql-port      (cdr (assoc :port params))) ; todo
-          (sql-database  (cdr (assoc :database params)))
-          (sql-user      (cdr (assoc :dbuser params)))
-          (sql-password  (cdr (assoc :dbpassword params)))
-          (prompt-regexp (sql-get-product-feature engine :prompt-regexp ))
-          sqli-buffer
-          rpt)
-      ;; Get credentials.
-      ;; either all fields are provided
-      ;; or there's a specific case were no login is needed
-      ;; or trigger the prompt
-      (or (and sql-database sql-user sql-server)
-          (eq sql-product 'sqlite) ;; sqlite allows in-memory db, w/o login
-          (apply #'sql-get-login
-                 (sql-get-product-feature engine :sqli-login)))
-      ;; depending on client, password is forcefully prompted
-
-      ;; The password wallet returns a function
-      ;; which supplies the password. (untested)
-      (when (functionp sql-password)
-        (setq sql-password (funcall sql-password)))
-
-      ;; Erase previous sql-buffer.
-      ;; Will look for it's prompt to indicate session readyness.
-      (let ((previous-session
-             (get-buffer (format "*SQL: %s*" sql-cnx))))
-        (when previous-session
-          (with-current-buffer
-              previous-session (erase-buffer)))
-
-        (setq sqli-buffer
-              (let ((process-environment (copy-sequence process-environment))
-                    (variables (plist-get org-sql-environment engine)))
-                (mapc (lambda (elem)   ; environment variables, evaluated here
-                        (setenv (car elem) (eval (cadr elem))))
-                      variables)
-                (funcall (sql-get-product-feature engine :sqli-comint-func)
-                         engine
-                         (sql-get-product-feature engine :sqli-options)
-                         (format "SQL: %s" sql-cnx))))
-        (setq sql-buffer (buffer-name sqli-buffer))
-
-        (setq rpt (sql-make-progress-reporter nil "Login"))
-        (with-current-buffer sql-buffer
-          (let ((proc (get-buffer-process sqli-buffer))
-                (secs org-sql-timeout)
-                (step 0.2))
-            (while (and proc
-                        (memq (process-status proc) '(open run))
-                        (or (accept-process-output proc step)
-                            (<= 0.0 (setq secs (- secs step))))
-                        (progn (goto-char (point-max))
-                               (not (re-search-backward
-                                     prompt-regexp 0 t))))
-              (sql-progress-reporter-update rpt)))
-
-          ;; no prompt, connexion failed (and process is terminated)
-          (goto-char (point-max))
-          (unless (re-search-backward prompt-regexp 0 t)
-            (user-error "Connection failed"))) ;is this a _user_ error?
-        ;;(run-hooks 'sql-login-hook) ; don't
-        )
-      (sql-progress-reporter-done rpt)
-      (get-buffer sqli-buffer))))
-
 (defun org-sql-session-format-query (str in-engine)
   "Process then send the command STR to the SQL process.
 Provide IN-ENGINE to retrieve product features.
--
2.39.5