Subject: Re: bug#23566: sql-sqlite: selecting database file is crippled)

From: Rolf Ade <[email-masked]>
Date: Sun, 01 Jan 2017 04:13:39 +0100

Still open with GNU Emacs 26.0.50.3. Lemme rephrase.

Emacs with sql-mode is a great environment in general, to do database
work with SQLite database files.

But there are currently 3 problems:
----------------------------------

- With emacs -Q, it is impossible to use SQLite databases with a name,
  that doesn't match ".*\\.\\(db\\|sqlite[23]?\\)". Contrariwise it
  should be possible to work with sqlite databases with any name.

- With emacs -Q, if the SQLite database file to work with isn't in the
  current directory then navigating to the file doesn't provide
  completion of directory names. The user has to type every character
  of the path by itself (with expection of very rare cases). This kind
  of completion is so ubiquitous, at least within emacs while
  selecting a file with the mini-buffer that you may not remeber
  anymore how boring it in fact is if this feature is missing.

- With emacs -Q it is not possible to create a new SQLite database
  with what sql-mode provides. There are clumsy work-arounds for this
  sql-mode limitation but that should not be necessary.

All this deficiencies are related, because they have their root in one
implementation detail (see below, part "Technical details").

How to reproduce:
-----------------

emacs -Q

M-x sql-sqlite RET

This prompts the user for the database file to use.

- Try to select a database file with a name, that does not match
  ".*\\.\\(db\\|sqlite[23]?\\)" and you see you can't.

- Try to select a database outside the current directory, using TAB
  completion for directory names on the way and you see, that it
  doesn't work (for directory names not matching the same name pattern
  as above, which is probably the overwhelming majority)

- Try to enter an up to now non-existing file (to create a new sqlite
  database and to work on that) and you see you can't.

This is just the simplest recipt to demonstrate the problems. There
are also other use pattern, which suffer from this (and there is no
use pattern, to avoid them).

Is it important?
----------------

For user of sql-mode w/ product SQLite: Definitely yes. It seriously
cripples the usefulness of sql-mode for SQLite databases.

I'd estimate that the majority of the users, that work with SQLite
database files with the help of sql-mode will be hindered by at least
one of the decribed problems more sooner than later (and therfore giving
up to use emacs for this). Even if the user only ever want to work with
database file names, that match the default name pattern and never want
to create a new SQLite database from within emacs it is unlikely, that
he only want to work with database files out of the current directory.

More background information (you may skip this part):
----------------------------------------------------

SQLite is the most widely deployed database engine in the world today.

SQLite stores a SQL database (with multiple tables, indices, triggers,
and views) in a single disk file. SQLite is meant as an embedded SQL
database but the distribution includes also a command line tool called
sqlite3, to access or modify such database files with SQL commands.

Due to SQLites widespread usage there are literally myriads of SQlite
database files out there on almost every thinkable plattform, which a
user may wish to explore or modify with the help of emacs and
sql-mode.

Technical Details
-----------------

Sql-mode uses the sqlite cli based on comint-mode. To call the cli
application, it needs the file name of the database. It request that
from the user with a read-file-name() call (in sql-get-login-ext()):

        (read-file-name prompt
                        (file-name-directory last-value) default t
                        (file-name-nondirectory last-value)
                        (when (plist-get plist :file)
                          `(lambda (f)
                             (string-match
                              (concat "\\<" ,(plist-get plist :file) "\\>")
                              (file-name-nondirectory f)))))))

The above desribed three problems result from the fourth and the sixth
argument of that read-file-name() call.

The t value of the fourth argument is the reason for the third problem
above.

The plist value at the point above is the value of
sql-sqlite-login-params, which is by default:

((database :file ".*\\.\\(db\\|sqlite[23]?\\)"))

So the read-file-name use the predicate lamda: it matches the file
name againts the regexp in sql-sqlite-login-params. This explanes the
problems one and two from above.

What to do?
-----------

Well, anything would be better, than what is now.

The least invasive change would be this small patch (for 26.0.50.3):

diff --git a/lisp/progmodes/sql.el b/lisp/progmodes/sql.el
index 9608a7d..1d9f123 100644
--- a/lisp/progmodes/sql.el
+++ b/lisp/progmodes/sql.el
@@ -933,7 +933,7 @@ sql-sqlite-options
   :version "20.8"
   :group 'SQL)
 
-(defcustom sql-sqlite-login-params '((database :file ".*\\.\\(db\\|sqlite[23]?\\)"))
+(defcustom sql-sqlite-login-params '((database :file nil))
   "List of login parameters needed to connect to SQLite."
   :type 'sql-login-params
   :version "24.1"
@@ -2954,7 +2954,7 @@ sql-get-login-ext
       ((plist-member plist :file)
        (expand-file-name
         (read-file-name prompt
-                        (file-name-directory last-value) default t
+                        (file-name-directory last-value) default 'confirm
                         (file-name-nondirectory last-value)
                         (when (plist-get plist :file)
                           `(lambda (f)

With this two changed lines and with emacs -Q all three described
problems are gone. Though, not really for good. If a user uses the whole
machinery and customize sql-sqlite-login-params, problem two would be
back.

Therefor I propose this slightly more elaborate patch:

diff --git a/lisp/progmodes/sql.el b/lisp/progmodes/sql.el
index 9608a7d..c3c61f2 100644
--- a/lisp/progmodes/sql.el
+++ b/lisp/progmodes/sql.el
@@ -933,7 +933,7 @@ sql-sqlite-options
   :version "20.8"
   :group 'SQL)
 
-(defcustom sql-sqlite-login-params '((database :file ".*\\.\\(db\\|sqlite[23]?\\)"))
+(defcustom sql-sqlite-login-params '((database :file nil))
   "List of login parameters needed to connect to SQLite."
   :type 'sql-login-params
   :version "24.1"
@@ -2954,13 +2954,15 @@ sql-get-login-ext
       ((plist-member plist :file)
        (expand-file-name
         (read-file-name prompt
-                        (file-name-directory last-value) default t
+                        (file-name-directory last-value) default 'confirm
                         (file-name-nondirectory last-value)
                         (when (plist-get plist :file)
                           `(lambda (f)
-                             (string-match
-                              (concat "\\<" ,(plist-get plist :file) "\\>")
-                              (file-name-nondirectory f)))))))
+                             (if (not(file-regular-p f))
+                                 t
+                               (string-match
+                                (concat "\\<" ,(plist-get plist :file) "\\>")
+                                (file-name-nondirectory f))))))))
 
       ((plist-member plist :completion)
        (completing-read prompt-def (plist-get plist :completion) nil t

Discussion of the proposed patch:
--------------------------------

There is no need for fear, a user may by mistake damage a non SQLite
file (by not restricting the file select to a small number of file name
suffixes (which would't be a great strategy. if such fear had ground).
Every SQLite database file has a 100 byte long database header described
here: https://www.sqlite.org/fileformat.html#the_database_header

At the first time the used cli programm sqlite3 has to read from or
write to the file, it checks this header bytes and refuses to alter the
file with an error msg ("Error: file is encrypted or is not a
database"). It is extremly unlikely (although, admitted, not completely
impossible), that a file starts with 100 bytes that make sense to
sqlite3, but is not a SQLite database file.

There is no strong convention amongst SQLite user about the suffix of
the database files. One user reported "One program I use, has 20
different suffixes, for the SQLite databases that it utilizes. Most of
its competitors use between 3 and 5 suffixes for the SQLite databases
that they utilize."
(https://www.mail-archive.com/[email-masked]/msg99952.html)

To some users, it may be helpful to customize sql-sqlite-login-params
(from its new default "complete every file name" by my proposed
patch). But to provide a default name pattern, that doesn't hinders
legitimate usage seems to be hopeless.

Customization of sql-sqlite-login-params is of course already possible
with current emacs. To work around the described problems in emacs I
have in my init.el:

(setq sql-sqlite-login-params '((database :file nil)))

This fixes the first two problems from above (the two most relevant).

But it is not only, that current emacs doesn't allow certain sane
operation for no good reason (using a SQLite database file with a
'wrong name') or lacks name completion while selecting a file with the
mini-buffer for a certain operation (emacs lacks completion???).

If a user notice this - and is an emacs enthusiast, beliving, that
there must be something, otherwise he would have give up on this tool
(emacs) for this purpose at this point - and start to tinker, it isn't
even obvious what the work-around is. E.g. the documentation of
sql-sqlite doesn't even mention sql-sqlite-login-params.

Rambling
--------

This miss-behaviour is annoying. It is unnecessary. It's a pity,
because otherwise sql-mode with SQLite database files is a fine tool.
It is in its core not about sql-mode architecture or its implementation
details, you don't need to know anything about that, its just about
knowing read-file-name and looking at that one occurrence of that in
sql.el.

I understand: thousands of open bugs and nobody actively maintaining
some files and all. Probably some kind soul could set a confirm tag,
so that I'm able to let rest the case, now that I've even provided
a patch. I've the customisation from above in my init.el, amongst
other cruft. I would love to remove that, somewhere in the next 20
years. Just for the few moments of the feeling, that my life had some
sense.