nanomdm icon indicating copy to clipboard operation
nanomdm copied to clipboard

MariaDB compatibility

Open alwatts opened this issue 2 years ago • 5 comments

Several Linux distributions have replaced MySQL in their repositories with MariaDB, a fork of MySQL. MariaDB does not support INSERT ... AS, so this PR rewrites the existing use of INSERT ... AS to a form that works in both MySQL and MariaDB.

alwatts avatar Feb 09 '23 03:02 alwatts

Thanks for this PR. The existing syntax is MySQL 8.x which is explicitly called out in the requirements. In #62 I mentioned:

Downgrading the syntax to MySQL 5.x isn't something we'll likely support (though — I might be open to adding a mysql5 backend. Maybe. Maybe not.

That said I'm willing to continue the conversation on this – either in the Slack channel or perhaps in #3 (or another GH discussion).

jessepeterson avatar Feb 09 '23 03:02 jessepeterson

To be clear: the reason for this stance is that we would be going backward with MySQL support. VALUES() is deprecated:

The use of VALUES() to refer to the new row and columns is deprecated beginning with MySQL 8.0.20, and is subject to removal in a future version of MySQL. Instead, use row and column aliases, as described in the next few paragraphs of this section.

jessepeterson avatar Feb 09 '23 17:02 jessepeterson

So, thinking about this, perhaps we could implement a sort of "dialects" option or something. I haven't tested this code, but something akin to this:

diff --git a/storage/mysql/mysql.go b/storage/mysql/mysql.go
index 166dfac..e26d17c 100644
--- a/storage/mysql/mysql.go
+++ b/storage/mysql/mysql.go
@@ -25,6 +25,8 @@ type MySQLStorage struct {
        logger log.Logger
        db     *sql.DB
        rm     bool
+
+       newOnDupStx bool
 }
 
 type config struct {
@@ -82,7 +84,7 @@ func New(opts ...Option) (*MySQLStorage, error) {
        if err = cfg.db.Ping(); err != nil {
                return nil, err
        }
-       return &MySQLStorage{db: cfg.db, logger: cfg.logger, rm: cfg.rm}, nil
+       return &MySQLStorage{db: cfg.db, logger: cfg.logger, rm: cfg.rm, newOnDupStx: true}, nil
 }
 
 // nullEmptyString returns a NULL string if s is empty.
@@ -129,6 +131,20 @@ func (s *MySQLStorage) storeDeviceTokenUpdate(r *mdm.Request, msg *mdm.TokenUpda
        return err
 }
 
        return err
 }
 
+func (s *MySQLStorage) asNew() string {
+       if s.newOnDupStx {
+               return " AS new"
+       }
+       return ""
+}
+
+func (s *MySQLStorage) asNewCol(col string) string {
+       if s.newOnDupStx {
+               return "new." + col
+       }
+       return "VALUES(" + col + ")"
+}
+
 func (s *MySQLStorage) storeUserTokenUpdate(r *mdm.Request, msg *mdm.TokenUpdate) error {
        // there shouldn't be an Unlock Token on the user channel, but
        // complain if there is to warn an admin
@@ -142,13 +158,13 @@ func (s *MySQLStorage) storeUserTokenUpdate(r *mdm.Request, msg *mdm.TokenUpdate
 INSERT INTO users
     (id, device_id, user_short_name, user_long_name, token_update, token_update_at)
 VALUES
-    (?, ?, ?, ?, ?, CURRENT_TIMESTAMP) AS new
+    (?, ?, ?, ?, ?, CURRENT_TIMESTAMP)`+s.asNew()+`
 ON DUPLICATE KEY
 UPDATE
     device_id = new.device_id,
-    user_short_name = new.user_short_name,
-    user_long_name = new.user_long_name,
-    token_update = new.token_update,
+    user_short_name = `+s.asNewCol("user_short_name")+`,
+    user_long_name = `+s.asNewCol("user_long_name")+`,
+    token_update = `+s.asNewCol("token_update")+`,
     token_update_at = CURRENT_TIMESTAMP;`,
                r.ID,
                r.ParentID,

Then of course hook up config and Options configurable in the backend options. Thoughts?

jessepeterson avatar Feb 21 '23 19:02 jessepeterson

@alwatts if this is a direction that seems okay (more 'dynamic' SQL queries) perhaps you want to give a go at #64 at the same time? No worries if not, though. :)

jessepeterson avatar Mar 17 '23 17:03 jessepeterson

@jessepeterson Can do! Happy to be assigned it.

alwatts avatar Mar 18 '23 02:03 alwatts