sql.el MariaDB support

sql.el MariaDB support

[Robert Cochran]

Hello emacs-devel,

I've decided that I want to spend a little bit of time improving the
MariaDB support in sql.el to make a nicer experience, namely changing
the prompt regexps so that the prompts will actually show up without
having to fiddle with sql-prompt-regexp.

What I'm not so sure about, and thus asking here, is whether or not to
put the MariaDB changes in with the MySQL product? MariaDB is still a
drop-in replacement for MySQL AFAIK, so in my mind it wouldn't be
unreasonable to lump those two together as a single product. But I'm not
well-versed on what doing that may imply.

TBH, I'd rather lump them together, as really all I'm intending to do is
modify the continuation prompt regexps to work for either database, and
making a whole separate MariaDB product would be overkill in my mind.

Thoughts?

-- 
~Robert Cochran

GPG Fingerprint - BD0C 5F8B 381C 64F0 F3CE  E7B9 EC9A 872C 41B2 77C2

Re: sql.el MariaDB support

[Eli Zaretskii]

> From: Robert Cochran <robert-emacs@cochranmail.com>
> Date: Tue, 05 Jun 2018 20:07:50 -0700
> 
> I've decided that I want to spend a little bit of time improving the
> MariaDB support in sql.el to make a nicer experience, namely changing
> the prompt regexps so that the prompts will actually show up without
> having to fiddle with sql-prompt-regexp.
> 
> What I'm not so sure about, and thus asking here, is whether or not to
> put the MariaDB changes in with the MySQL product? MariaDB is still a
> drop-in replacement for MySQL AFAIK, so in my mind it wouldn't be
> unreasonable to lump those two together as a single product. But I'm not
> well-versed on what doing that may imply.
> 
> TBH, I'd rather lump them together, as really all I'm intending to do is
> modify the continuation prompt regexps to work for either database, and
> making a whole separate MariaDB product would be overkill in my mind.
> 
> Thoughts?

I hope Micheal (CC'ed) will have comments on your proposal.

Thanks.

Re: sql.el MariaDB support

[Michael Mauger]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On June 6, 2018 10:53 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> ​​
> > From: Robert Cochran robert-emacs@cochranmail.com
> > Date: Tue, 05 Jun 2018 20:07:50 -0700
> > 
> > I've decided that I want to spend a little bit of time improving the
> > MariaDB support in sql.el to make a nicer experience, namely changing
> > the prompt regexps so that the prompts will actually show up without
> > having to fiddle with sql-prompt-regexp.
> > What I'm not so sure about, and thus asking here, is whether or not to
> > put the MariaDB changes in with the MySQL product? MariaDB is still a
> > drop-in replacement for MySQL AFAIK, so in my mind it wouldn't be
> > unreasonable to lump those two together as a single product. But I'm not
> > well-versed on what doing that may imply.
> > TBH, I'd rather lump them together, as really all I'm intending to do is
> > modify the continuation prompt regexps to work for either database, and
> > making a whole separate MariaDB product would be overkill in my mind.
> > 
> > Thoughts?
> 
> I hope Micheal (CC'ed) will have comments on your proposal.

Great idea! My recommendation is to create a new sql product/dialect akin 
to mysql in sql.el and either reuse the mysql settings/variables where 
appropriate, or create defvaralias's for the mariadb variables pointing back
to the mysql variables. 

That is, reuse the font-locking and login setup, but override the prompt string.

I'd also encourage you to update the shared font-lock settings to get it 
up-to-date with the latest features. It's been a very long time since I used 
MySQL so there is some bit-rot there.

Feel free to contact me with additional questions, and Happy Hacking!

PS. If you haven't already, I'd look into getting your copyright paperwork
done so that the Emacs project can easily accept your patches.
​-- 
MICHAEL@MAUGER.COM // FSF and EFF member // GNU Emacs sql.el maintainer​

Re: sql.el MariaDB support

[Eli Zaretskii]

> Date: Wed, 06 Jun 2018 13:16:20 -0400
> From: Michael Mauger <mmauger@protonmail.com>
> Cc: Robert Cochran <robert-emacs@cochranmail.com>, "emacs-devel@gnu.org" <emacs-devel@gnu.org>
> 
> PS. If you haven't already, I'd look into getting your copyright paperwork
> done so that the Emacs project can easily accept your patches.

Robert's copyright assignment is on file already.

Re: sql.el MariaDB support

[Michael Mauger]

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

-------- Original Message --------
> On Jun 6, 2018, 1:28 PM, Eli Zaretskii wrote:
>
> > Date: Wed, 06 Jun 2018 13:16:20 -0400
> > From: Michael Mauger
> > Cc: Robert Cochran , "emacs-devel@gnu.org"
> >
> > PS. If you haven't already, I'd look into getting your copyright paperwork
> > done so that the Emacs project can easily accept your patches.
>
> Robert's copyright assignment is on file already.
>

Great! I just like making sure that things are in order before they start complaining that I won't commit their changes. If he's all set, then nothing to worry about. Thx

[-- Attachment #2: Type: text/html, Size: 712 bytes --]

Re: sql.el MariaDB support

[Robert Cochran]

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

Michael Mauger <mmauger@protonmail.com> writes:

> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On June 6, 2018 10:53 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> ​​
>> > From: Robert Cochran robert-emacs@cochranmail.com
>> > Date: Tue, 05 Jun 2018 20:07:50 -0700
>> > 
>> > I've decided that I want to spend a little bit of time improving the
>> > MariaDB support in sql.el to make a nicer experience, namely changing
>> > the prompt regexps so that the prompts will actually show up without
>> > having to fiddle with sql-prompt-regexp.
>> > What I'm not so sure about, and thus asking here, is whether or not to
>> > put the MariaDB changes in with the MySQL product? MariaDB is still a
>> > drop-in replacement for MySQL AFAIK, so in my mind it wouldn't be
>> > unreasonable to lump those two together as a single product. But I'm not
>> > well-versed on what doing that may imply.
>> > TBH, I'd rather lump them together, as really all I'm intending to do is
>> > modify the continuation prompt regexps to work for either database, and
>> > making a whole separate MariaDB product would be overkill in my mind.
>> > 
>> > Thoughts?
>> 
>> I hope Micheal (CC'ed) will have comments on your proposal.
>
> Great idea! My recommendation is to create a new sql product/dialect akin 
> to mysql in sql.el and either reuse the mysql settings/variables where 
> appropriate, or create defvaralias's for the mariadb variables pointing back
> to the mysql variables. 
>
> That is, reuse the font-locking and login setup, but override the prompt string.

Alright, I went that route. I made a copy of the MySQL entry and
modified it to fit MariaDB. I then made most all of the MariaDB
variables defvaralias-es of the MySQL ones. `sql-comint-mariadb' just
calls into `sql-comint-mysql'. MariaDB is almost no different than MySQL
except for the prompt.

> I'd also encourage you to update the shared font-lock settings to get it 
> up-to-date with the latest features. It's been a very long time since I used 
> MySQL so there is some bit-rot there.

I'm probably not your guy on this one. I'm a *very* casual user, so I'd
honestly have no idea where to even start with that. Sorry. :(

So my first attempt is attached below. Doubtless I've done at least a
handful of things wrong, but I can just fix them as we go.

A couple of things I've been thinking about that might be wrong/need
changing:

1) Should we try to support a configuration where the MariaDB and MySQL
configuration options differ (aka making MariaDB variables not mere
aliases for the MySQL ones)? Simple-minded me thinks not, that there
wouldn't really be a point in doing that since MariaDB is supposed to be
able to drop-in replace MySQL. But I'm asking anyways.

2) Will it be problematic that the MariaDB entry doesn't have a
:prompt-length? The MariaDB prompt contains the current database, so
there's no real knowing ahead of time how long the prompt will
be. Looking at how it's used, it appears that query text on continuation
lines will be messed up. Maybe it would be close enough to have
:prompt-length be (length "MariaDB []> ") - everything except the
database name?

3) To that end, is it okay that my regexp for matching the database name
is pretty loose? I figured on the first pass that being a little
generous with the match criterion is acceptable, but I wouldn't be
surprised if that wasn't good enough for a final solution.

Well, here goes. Hopefully this won't need tons of rework cycles.

HTH,
-- 
~Robert Cochran

GPG Fingerprint - BD0C 5F8B 381C 64F0 F3CE  E7B9 EC9A 872C 41B2 77C2



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Add MariaDB support to sql.el --]
[-- Type: text/x-patch, Size: 4108 bytes --]

From e56a3658a809e6737c46930c2387817f97ce3d8f Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@cochranmail.com>
Date: Thu, 7 Jun 2018 01:13:47 -0700
Subject: [PATCH] Add MariaDB support to lisp/progmodes/sql.el

* lisp/progmodes/sql.el (sql-product-alist): Add MariaDB entry
  (sql-mariadb-program), (sql-mariadb-options),
  (sql-mariadb-login-params), (sql-mode-mariadb-font-lock): New
  variables, aliases of the MySQL equivalents
  (sql-mariadb), (sql-comint-mariadb): New interaction mode
  functions for MariaDB
---
 lisp/progmodes/sql.el | 68 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/lisp/progmodes/sql.el b/lisp/progmodes/sql.el
index 63428610a5..bc30fc01a6 100644
--- a/lisp/progmodes/sql.el
+++ b/lisp/progmodes/sql.el
@@ -416,6 +416,23 @@ sql-product-alist
      :prompt-regexp "^SQL>"
      :prompt-length 4)
 
+    (mariadb
+     :name "MariaDB"
+     :free-software t
+     :font-lock sql-mode-mariadb-font-lock-keywords
+     :sqli-program sql-mariadb-program
+     :sqli-options sql-mariadb-options
+     :sqli-login sql-mariadb-login-params
+     :sqli-comint-func sql-comint-mariadb
+     :list-all "SHOW TABLES;"
+     :list-table "DESCRIBE %s;"
+     ;; FIXME: I really don't know what the proper regexp for valid
+     ;; database names is, so just match anything
+     :prompt-regexp "^MariaDB \\[.*]> "
+     :prompt-cont-regexp "^    [\"'`-]> "
+     :syntax-alist ((?# . "< b"))
+     :input-filter sql-remove-tabs-filter)
+
     (ms
      :name "Microsoft"
      :font-lock sql-mode-ms-font-lock-keywords
@@ -982,6 +999,17 @@ sql-mysql-login-params
   :version "24.1"
   :group 'SQL)
 
+;; Customization for MariaDB
+
+;; MariaDB is a drop-in replacement for MySQL, so just make the
+;; MariaDB variables aliases of the MySQL ones.
+
+(defvaralias 'sql-mariadb-program 'sql-mysql-program)
+
+(defvaralias 'sql-mariadb-options 'sql-mysql-options)
+
+(defvaralias 'sql-mariadb-login-params 'sql-mysql-login-params)
+
 ;; Customization for Solid
 
 (defcustom sql-solid-program "solsql"
@@ -2399,6 +2427,8 @@ sql-mode-mysql-font-lock-keywords
 function `regexp-opt'.  Therefore, take a look at the source before
 you define your own `sql-mode-mysql-font-lock-keywords'.")
 
+(defvaralias 'sql-mode-mariadb-font-lock-keywords 'sql-mode-mysql-font-lock-keywords)
+
 (defvar sql-mode-sqlite-font-lock-keywords
   (eval-when-compile
     (list
@@ -4875,6 +4905,44 @@ sql-comint-mysql
               (list sql-database)))))
     (sql-comint product params buf-name)))
 
+;;;###autoload
+(defun sql-mariadb (&optional buffer)
+    "Run mariadb by TcX as an inferior process.
+
+MariaDB is free software.
+
+If buffer `*SQL*' exists but no process is running, make a new process.
+If buffer exists and a process is running, just switch to buffer
+`*SQL*'.
+
+Interpreter used comes from variable `sql-mariadb-program'.  Login uses
+the variables `sql-user', `sql-password', `sql-database', and
+`sql-server' as defaults, if set.  Additional command line parameters
+can be stored in the list `sql-mariadb-options'.
+
+The buffer is put in SQL interactive mode, giving commands for sending
+input.  See `sql-interactive-mode'.
+
+To set the buffer name directly, use \\[universal-argument]
+before \\[sql-mariadb].  Once session has started,
+\\[sql-rename-buffer] can be called separately to rename the
+buffer.
+
+To specify a coding system for converting non-ASCII characters
+in the input and output to the process, use \\[universal-coding-system-argument]
+before \\[sql-mariadb].  You can also specify this with \\[set-buffer-process-coding-system]
+in the SQL buffer, after you start the process.
+The default comes from `process-coding-system-alist' and
+`default-process-coding-system'.
+
+\(Type \\[describe-mode] in the SQL buffer for a list of commands.)"
+  (interactive "P")
+  (sql-product-interactive 'mariadb buffer))
+
+(defun sql-comint-mariadb (product options &optional buf-name)
+  "Create comint buffer and connect to MariaDB."
+  (sql-comint-mysql proudct options buf-name))
+
 \f
 
 ;;;###autoload
-- 
2.17.1

Re: sql.el MariaDB support

[Michael Mauger]

On June 7, 2018 4:48 AM, Robert Cochran <robert-emacs@cochranmail.com> wrote:
> ​​
> ... I made a copy of the MySQL entry and
> modified it to fit MariaDB. I then made most all of the MariaDB
> variables defvaralias-es of the MySQL ones. `sql-comint-mariadb' just calls into`sql-comint-mysql'. 
> MariaDB is almost no different than MySQL except for the prompt.
> 

I had to move the mariadb defvaraliases before the mysql defvar because that is the order expected. 

> So my first attempt is attached below. Doubtless I've done at least a
> handful of things wrong, but I can just fix them as we go.
> 
> A couple of things I've been thinking about that might be wrong/need
> changing:
> 
> 1.  Should we try to support a configuration where the MariaDB and MySQL
>     configuration options differ (aka making MariaDB variables not mere
>     aliases for the MySQL ones)? Simple-minded me thinks not, that there
>     wouldn't really be a point in doing that since MariaDB is supposed to be
>     able to drop-in replace MySQL. But I'm asking anyways.
>     

This is obviously the potential gotcha here, but if you are flipping between the two,
you have enough other problems that I don't think Emacs will be you biggest. :)

> 2.  Will it be problematic that the MariaDB entry doesn't have a
>     :prompt-length? The MariaDB prompt contains the current database, so
>     there's no real knowing ahead of time how long the prompt will
>     be. Looking at how it's used, it appears that query text on continuation
>     lines will be messed up. Maybe it would be close enough to have
>     :prompt-length be (length "MariaDB []> ") - everything except the
>     database name?
> 

That's a hold-over from the original and I'm not sure it has any impact today. Let
me know if you have any problems...

> 3) To that end, is it okay that my regexp for matching the database name
>     is pretty loose? I figured on the first pass that being a little
>     generous with the match criterion is acceptable, but I wouldn't be
>     surprised if that wasn't good enough for a final solution.
> 

Keeping it loose is okay, anchoring it to the start of the line thru the "> " text is
really all that's needed.

> Well, here goes. Hopefully this won't need tons of rework cycles.
> 

I had to rearrange the declarations a bit and correct a couple of spelling/cut-n-paste 
issues, but it looks fine. I went ahead and committed the code and gave you credit.

> > I'd also encourage you to update the shared font-lock settings to get it
> > up-to-date with the latest features. It's been a very long time since I used
> > MySQL so there is some bit-rot there.
> 
> I'm probably not your guy on this one. I'm avery casual user, so I'd
> honestly have no idea where to even start with that. Sorry. :(
> 

I took care of this. Basically the font-lock variables have long lists of keywords, 
functions, and data types that are converted at build time to a massive regexp.
I updated the lists from the MySQL and MariaDB documentation. Let me know if 
you find any of the highlighting to be distracting or seeming incorrect. Without a 
serious increase in complexity of the matching, there will always be some odd cases.

> 
> ~Robert Cochran

Thank you very much, I greatly appreciate your contribution.

Happy Hacking!
​-- 
MICHAEL@MAUGER.COM // FSF and EFF member // GNU Emacs sql.el maintainer​

Re: sql.el MariaDB support

[Robert Cochran]

Michael Mauger <mmauger@protonmail.com> writes:

> I had to move the mariadb defvaraliases before the mysql defvar because that is the order expected. 

Oops. I mean, I had a 50/50 of getting it right on the first try, so I
suppose I didn't do too badly.

> This is obviously the potential gotcha here, but if you are flipping between the two,
> you have enough other problems that I don't think Emacs will be you biggest. :)

Good. I was thinking the exact same thing, but I was playing it safe by
asking the question and confirming whether or not that was considered
reasonable behavior.

> That's a hold-over from the original and I'm not sure it has any impact today. Let
> me know if you have any problems...

Alright. It doesn't particularly bother /me/ to not have that
aligned. I'm simply happy not to have to fiddle with the prompt regexps
every time I want to use the SQL REPL. :)

> Keeping it loose is okay, anchoring it to the start of the line thru the "> " text is
> really all that's needed.

Awesome, because I honestly wasn't really in the mood for trying to
concoct a complicated regexp to match all the possibilities. ;)

> I had to rearrange the declarations a bit and correct a couple of spelling/cut-n-paste 
> issues, but it looks fine. I went ahead and committed the code and gave you credit.

Thanks! Please forgive my n00b question: where exactly did it go? I
don't see it in the Emacs git, neither master nor emacs-26. Perhaps I'm
not searching for it correctly?

> I took care of this. Basically the font-lock variables have long lists of keywords, 
> functions, and data types that are converted at build time to a massive regexp.
> I updated the lists from the MySQL and MariaDB documentation. Let me know if 
> you find any of the highlighting to be distracting or seeming incorrect. Without a 
> serious increase in complexity of the matching, there will always be some odd cases.

Thanks. Perhaps I could have phrased it better - I'm a casual user WRT
SQL in general. I understood once I saw the list of keywords what was
eventually going to happen to the list of words. I'm not entirely sure
why I didn't grok 'add the new keywords to the list' - I somehow got a
mental image of a much more drastic undertaking. I could have done that
if I had been able to read properly! :)
Sorry!

> Thank you very much, I greatly appreciate your contribution.

Thank you for taking it.

> Happy Hacking!

And to you as well.

-- 
~Robert Cochran

GPG Fingerprint - BD0C 5F8B 381C 64F0 F3CE  E7B9 EC9A 872C 41B2 77C2

Re: sql.el MariaDB support

[Michael Mauger]

On June 11, 2018 9:10 PM, Robert Cochran <robert-emacs@cochranmail.com> wrote:
> 
> Thanks! Please forgive my n00b question: where exactly did it go? I
> don't see it in the Emacs git, neither master nor emacs-26. Perhaps I'm
> not searching for it correctly?

Nope, you were right, I had committed the change but hadn't pushed it; 
I forget that step more regularly than I'd like.

> > Thank you very much, I greatly appreciate your contribution.

> Thank you for taking it.

You're contribution was straightforward and helpful. If nothing else it prompted me
to spend more time looking at MySQL/MariaDB support. It is greatly appreciated.
 
​-- 
MICHAEL@MAUGER.COM // FSF and EFF member // GNU Emacs sql.el maintainer​