Support MySQL 8.0.19+ and suport and require mysql-connector-python 8…

….0.32

CHANGES.rst

@@ -10,10 +10,9 @@
 - Add the "Requires Python" metadata to prevent installation on Python
   < 3.8.
 - Add support for Python 3.11.
-- Bump tested database drivers to their latest versions, with the
-  exception of ``mysql-connector-python``; this driver is only tested
-  at version 8.0.31 as there are known incompatibilities with 8.0.32
-  (which is currently the latest version).
+- Bump tested database drivers to their latest versions. In
+  particular, the ``mysql-connector-python`` supported version is now
+  8.0.32, which introduces charset changes.
 - pg8000: Require 1.29.0. See :issue:`495`.
 - Fix the SQLite ZODB URI resolver. The ``data_dir`` query parameter
   replaces the ``path`` query parameter.
@@ -24,6 +23,14 @@
   revision of the object accessible. Previously, the most recent
   revision of the object became unavailable. See :pr:`484`, with
   thanks to Kirill Smelkov.
+- Add support for MySQL 8.0.20 and above. In version 8.0.19, MySQL
+  deprecated the traditional ``SET col = VALUES(col)`` upsert syntax
+  in favor of a more PostgreSQL like ``SET col = excluded.col``
+  syntax. In version 8.0.20, MySQL started issuing warnings about the
+  older syntax, and in certain database drivers (MySQL
+  Connector/Python 8.0.32+) these warnings became ``TypeError`` exceptions
+  (due to a bug in the driver). Now, we use the new syntax on versions
+  that support it.
 
 3.5.0 (2022-09-16)
 ==================

setup.py

@@ -273,9 +273,8 @@ def read_file(*path):
             # mysqlclient (binary) on all CPythons. It's the default.
             'mysqlclient >= 2.0.0',
             # mysql-connector-python; one of two pure-python versions
-            # XXX: >= 8.0.32 has issues with binary values!
             # This requirement is repeated in the driver class.
-            'mysql-connector-python == 8.0.31; python_version == "3.10"',
+            'mysql-connector-python >= 8.0.32; python_version == "3.10"',
 
             # postgresql
             # pure-python

src/relstorage/adapters/mysql/adapter.py

@@ -142,6 +142,7 @@ def _create(self):
             self.mover = MySQLObjectMover(
                 driver,
                 options=options,
+                version_detector=self.version_detector,
             )
 
         if self.oidallocator is None:

src/relstorage/adapters/mysql/drivers/__init__.py

@@ -55,16 +55,56 @@ def _quote_query_for_prepare(self, query):
     # more similar to what PostgreSQL uses, possibly allowing more
     # query sharing (with a smarter query runner/interpreter).
 
-
     def visit_upsert_conflict_column(self, _column):
         self.emit_keyword('ON DUPLICATE KEY')
 
     def visit_upsert_conflict_update(self, update):
         self.emit_keyword('UPDATE')
         self.visit_csv(update.col_expressions)
 
+    def visit_upsert_values(self, values):
+        # If we are on 8.0.19 or newer,
+        # we need to emit the 'excluded' alias.
+
+        # By the time we're compiling statements, we've already
+        # initialized the schema, which means that we queried
+        # the version of the database, which means that we can
+        # use the version detector without passing in a cursor
+        ver_det = values.context.version_detector
+        if ver_det.requires_values_upsert_alias(None):
+            self.emit_w_padding_space('AS excluded')
+
     def visit_upsert_excluded_column(self, column):
-        self.emit('VALUES(%s)' % (column.name,))
+        # see visit_upsert_values
+        ver_det = column.context.version_detector
+        if ver_det.requires_values_upsert_alias(None):
+            super().visit_upsert_excluded_column(column)
+        else:
+            # Prior to 8.0.19
+            self.emit('VALUES(%s)' % (column.name,))
+
+    def visit_upsert_before_select(self, select):
+        # See the previous visit_upsert methods.
+        # Here, we have to transform
+        #
+        # INSERT INTO t(c)
+        # SELECT c FROM foo
+        # ON DUPLICATE KEY UPDATE c = VALUES(foo)
+        #
+        # into a nested subquery that we can name:
+        #
+        # INSERT INTO t(c)
+        # SELECT * FROM (SELECT c FROM foo) AS excluded
+        # ON DUPLICATE KEY UPDATE c = VALUES(foo)
+
+        ver_det = select.context.version_detector
+        if ver_det.requires_values_upsert_alias(None):
+            self.emit('SELECT * FROM (')
+
+    def visit_upsert_after_select(self, select):
+        ver_det = select.context.version_detector
+        if ver_det.requires_values_upsert_alias(None):
+            self.emit(') AS excluded')
 
 
 class MySQLDialect(DefaultDialect):
@@ -84,6 +124,7 @@ class MySQLDialect(DefaultDialect):
     def compiler_class(self):
         return MySQLCompiler
 
+
 class IterateFetchmanyMixin(object):
     """
     Mixin to cause us to fetch in batches using fetchmany().
@@ -102,6 +143,7 @@ def __iter__(self):
 
     next = __next__ = None
 
+
 class AbstractMySQLDriver(AbstractModuleDriver):
 
     MY_SESSION_VARS = {

src/relstorage/adapters/mysql/drivers/mysqlconnector.py

@@ -44,7 +44,10 @@ class PyMySQLConnectorDriver(AbstractMySQLDriver):
     PRIORITY = 4
     PRIORITY_PYPY = 2
     REQUIREMENTS = (
-        'mysql-connector-python == 8.0.31',
+        # 8.0.32 changes character set handling,
+        # adds init_command
+
+        'mysql-connector-python >= 8.0.32',
     )
 
 
@@ -58,15 +61,12 @@ def __init__(self):
         super().__init__()
         # This driver doesn't support ``init_command``, we have to run
         # it manually.
-        self.__init_command = self._init_command
-        del self._init_command
+        #self.__init_command = self._init_command
+        #del self._init_command
 
         # conn.close() -> InternalError: Unread result found
         # By the time we get to a close(), it's too late to do anything about it.
         self.close_exceptions += (self.driver_module.InternalError,)
-        if self.Binary is str:
-            self.Binary = bytearray
-
         self.mysql_deadlock_exc = self.driver_module.DatabaseError
 
         if PYPY:
@@ -120,9 +120,6 @@ def _get_converter_class(cls):
 
             # The Python implementation calls row_to_python(), the C version
             # calls to_python()
-
-            # XXX: It's possible this area is where the problems on
-            # mysql.connector >= 8.0.33 are introduced.
             class BlobConverter(mysql.connector.conversion.MySQLConverter):
                 # There are a few places we get into trouble on
                 # Python 2/3 with bytearrays coming back: they
@@ -152,7 +149,28 @@ def connect(self, *args, **kwargs):
         # returns bytearray under Py2
         kwargs['use_pure'] = self.USE_PURE
         kwargs['converter_class'] = self._get_converter_class()
-        kwargs['get_warnings'] = True
+
+        # Prior to 8.0.32, we fetched and displayed warnings.
+        #
+        # But 8.0.32 is MESSED UP. First, it tries to use
+        # ``warnings.warn(integer)``, which raises a TypeError. Second
+        # and most importantly, there is no combination of
+        # ``charecter_set_*`` settings that let it work without
+        # warnings and/or errors. If we set those to ``utf8mb4``, then
+        # when we try to send pickle values we get a warning "(1300)
+        # Invalid utf8mb4 character string: '800363'" (and because of
+        # bug number one, this becomes an Exception). And if we set
+        # them to ``binary`` then when we try to send JSON values we
+        # get a hard error from the DB: "(22032): Cannot create a JSON
+        # value from a string with CHARACTER SET 'binary'" --- this is
+        # despite installing the procs with that character set.
+        #
+        # PyMySQL handles this by using the ``_binary`` character set
+        # introducer, but I don't find a way to insert that here. The
+        # only workaround I've found is to just ignore the warnings...
+
+        kwargs['get_warnings'] = False
+
         # By default, make it fetch all rows for the cursor, like most
         # drivers do. Unless we do this, cursors won't buffer, so we
         # don't know how many rows there are. That's fine and within
@@ -162,13 +180,7 @@ def connect(self, *args, **kwargs):
         # prepared cursor, but the prepared cursor doesn't gain us
         # anything anyway.
         kwargs['buffered'] = True
-        conn = super().connect(*args, **kwargs)
-        cur = conn.cursor()
-        try:
-            cur.execute(self.__init_command)
-        finally:
-            cur.close()
-        return conn
+        return super().connect(*args, **kwargs)
 
     def cursor(self, conn, server_side=False):
         if server_side:

src/relstorage/adapters/mysql/drivers/pymysql.py

@@ -53,7 +53,6 @@ class PyMySQLDriver(AbstractMySQLDriver):
 
     def __init__(self):
         super().__init__()
-
         pymysql_err = self.driver_module
 
         # Under PyMySql through at least 0.6.6, closing an already closed

src/relstorage/adapters/mysql/mover.py

@@ -95,21 +95,30 @@ def restore(self, cursor, batcher, oid, tid, data):
 
         Used for copying transactions into this database.
         """
+        if self.version_detector.requires_values_upsert_alias(cursor):
+            def VALUES(col):
+                return 'NEW.' + col
+        else:
+            def VALUES(col):
+                return f'VALUES({col})'
+
         if self.keep_history:
-            suffix = """
+            suffix = f"""
+            AS NEW
             ON DUPLICATE KEY UPDATE
-                tid = VALUES(tid),
-                prev_tid = VALUES(prev_tid),
-                md5 = VALUES(md5),
-                state_size = VALUES(state_size),
-                state = VALUES(state)
+                tid = {VALUES('tid')},
+                prev_tid = {VALUES('prev_tid')},
+                md5 = {VALUES('md5')},
+                state_size = {VALUES('state_size')},
+                state = {VALUES('state')}
             """
         else:
-            suffix = """
+            suffix = f"""
+            AS NEW
             ON DUPLICATE KEY UPDATE
-                tid = VALUES(tid),
-                state_size = VALUES(state_size),
-                state = VALUES(state)
+                tid = {VALUES('tid')},
+                state_size = {VALUES('state_size')},
+                state = {VALUES('state')}
             """
         self._generic_restore(batcher, oid, tid, data,
                               command='INSERT', suffix=suffix)

src/relstorage/adapters/mysql/schema.py

@@ -255,6 +255,8 @@ def create_procedures(self, cursor): # pylint:disable=too-many-locals
         # On ``INSERT INTO object_state...`` where the values are all parameterized
         # and are all bytstrings
         cursor.execute('SET SESSION character_set_client = utf8mb4, '
+                       'character_set_connection = utf8mb4, '
+                       'character_set_results = utf8mb4, '
                        'collation_connection = utf8mb4_general_ci')
 
         for name, create_stmt in self.procedures.items():
@@ -269,12 +271,13 @@ def create_procedures(self, cursor): # pylint:disable=too-many-locals
                 FOR_SHARE=for_share
             )
             assert checksum in create_stmt
+
             if name in installed:
                 installed_proc = installed[name]
                 stored_checksum = installed_proc.checksum
                 character_set_client = installed_proc.character_set_client
                 collation_connection = installed_proc.collation_connection
-                expected = (checksum, 'utf8', 'utf8_general_ci')
+                expected = (checksum, 'utf8mb4', 'utf8mb4_general_ci')
                 if expected != (stored_checksum, character_set_client, collation_connection):
                     logger.info(
                         "Re-creating procedure %s due to mismatch %s != %s",
@@ -366,3 +369,17 @@ def supports_good_stored_procs(self, cursor):
         See https://github.com/zodb/relstorage/pull/287#issuecomment-515518727
         """
         return self.get_version_info(cursor) >= (5, 7, 19)
+
+    def requires_values_upsert_alias(self, cursor):
+        """
+        Do we need to use ``INSERT INTO t(c1) VALUES (%s) AS
+        EXCLUDED``?
+
+        This is available on 8.0.19, and absolutely needed on 8.0.20,
+        else we get warnings from the database, which some drivers
+        like to turn into Python warnings (and which they get wrong
+        --- I'm looking at you, MySQLConnector/Python; it causes a
+        type error because the warning object it tries to warn has an
+        integer error code as its first arg, which causes a TyeError)
+        """
+        return self.get_version_info(cursor) >= (8, 0, 19)

src/relstorage/adapters/sql/dialect.py

@@ -290,6 +290,9 @@ def emit_null(self):
         self.emit('NULL')
 
     def emit_w_padding_space(self, value):
+        """
+        Emits the *value*, being sure it is surrounded by whitespace.
+        """
         ended_in_space = self.buf.getvalue().endswith(' ')
         value = value.strip()
         if not ended_in_space:
@@ -463,19 +466,49 @@ def visit_upsert_conflict_update(self, update):
         self.visit(update)
 
     def visit_upsert_excluded_column(self, column):
+        """
+        Called for each column on the right-hand-side of
+        ``ON DUPLICATE KEY UPDATE c = <EXCLUDED>``
+        (i.e., for the ``<EXCLUDED>`` portion).
+
+        By default, we use the PostgreSQL syntax to
+        refer to these columns using the 'excluded.'
+        table alias.
+        """
         self.emit('excluded.')
         self.emit(column.name)
 
+    def visit_upsert_values(self, values):
+        """
+        Called after emitting a ``VALUES ...`` clause for upserting.
+
+        We will be constructing SQL that basically looks like this::
+
+            INSERT INTO table (c1, c2)
+            VALUES (%s, %s), (%s, %s)  ......
+            ON DUPLICATE KEY UPDATE    ^^^^^^
+               .....
+
+        The location of the carets is where we are located in the
+        construction phase. This method is called to allow
+        setting a table alias for the VALUES clause. (Needed in MySQL 8.0.19
+        and above.) The default implementation does nothing,
+        but if you assign a table alias, it's good to use "excluded",
+        as that matches what PostgreSQL does.
+        """
+
     def visit_upsert_after_select(self, select): # pylint:disable=unused-argument
         pass
 
-class _DefaultContext(object):
+    def visit_upsert_before_select(self, select): # pylint:disable=unused-argument
+        pass
 
+
+class _DefaultContext(object):
     keep_history = True
 
 
 class DialectAware(object):
-
     context = _DefaultContext()
     dialect = _MissingDialect()
 
@@ -512,13 +545,20 @@ def bind(self, context, dialect=None):
             dialect = self._find_dialect(context)
 
         assert dialect is not None
-
         new = copy(self)
         # pylint:disable-next=protected-access
         return new._bound_to(context, dialect)
 
     _bind_vars_ignored = ()
 
+    def _get_vars_to_consider_binding(self):
+        """
+        Return an iterable of key/value pairs, where
+        key is the name of the attribute; the value will
+        be checked to see if it should be bound.
+        """
+        return vars(self).items()
+
     def _bound_to(self, context, dialect):
         # Called on the copy of self.
         if context is not None:
@@ -528,7 +568,7 @@ def _bound_to(self, context, dialect):
         bound_replacements = {
             k: v.bind(context, dialect)
             for k, v
-            in vars(self).items()
+            in self._get_vars_to_consider_binding()
             if k not in self._bind_vars_ignored and isinstance(v, DialectAware)
         }
         for k, v in bound_replacements.items():