Fixed rendering of index expressions in MySQL

Fixed Python-side autogenerate rendering of index expressions in MySQL
dialect by aligning it with SQLAlchemy's MySQL index expression rules. Pull
request courtesy david-fed.

Fixes: #1492
Closes: #1695
Pull-request: #1695
Pull-request-sha: 8f9ed8f

Change-Id: I3b838b4b7a44e3d5a279ba30624c1552f99959d7

Author: david-fed

alembic/ddl/mysql.py

@@ -11,6 +11,9 @@
 
 from sqlalchemy import schema
 from sqlalchemy import types as sqltypes
+from sqlalchemy.sql import elements
+from sqlalchemy.sql import functions
+from sqlalchemy.sql import operators
 
 from .base import alter_table
 from .base import AlterColumn
@@ -31,6 +34,7 @@
 
     from sqlalchemy.dialects.mysql.base import MySQLDDLCompiler
     from sqlalchemy.sql.ddl import DropConstraint
+    from sqlalchemy.sql.elements import ClauseElement
     from sqlalchemy.sql.schema import Constraint
     from sqlalchemy.sql.type_api import TypeEngine
 
@@ -47,6 +51,31 @@ class MySQLImpl(DefaultImpl):
     )
     type_arg_extract = [r"character set ([\w\-_]+)", r"collate ([\w\-_]+)"]
 
+    def render_ddl_sql_expr(
+        self,
+        expr: ClauseElement,
+        is_server_default: bool = False,
+        is_index: bool = False,
+        **kw: Any,
+    ) -> str:
+        # apply Grouping to index expressions;
+        # see https://github.com/sqlalchemy/sqlalchemy/blob/
+        # 36da2eaf3e23269f2cf28420ae73674beafd0661/
+        # lib/sqlalchemy/dialects/mysql/base.py#L2191
+        if is_index and (
+            isinstance(expr, elements.BinaryExpression)
+            or (
+                isinstance(expr, elements.UnaryExpression)
+                and expr.modifier not in (operators.desc_op, operators.asc_op)
+            )
+            or isinstance(expr, functions.FunctionElement)
+        ):
+            expr = elements.Grouping(expr)
+
+        return super().render_ddl_sql_expr(
+            expr, is_server_default=is_server_default, is_index=is_index, **kw
+        )
+
     def alter_column(
         self,
         table_name: str,

docs/build/unreleased/1492.rst

@@ -0,0 +1,7 @@
+.. change::
+    :tags: bug, mysql
+    :tickets: 1492
+
+    Fixed Python-side autogenerate rendering of index expressions in MySQL
+    dialect by aligning it with SQLAlchemy's MySQL index expression rules. Pull
+    request courtesy david-fed.

tests/test_mysql.py

@@ -6,6 +6,7 @@
 from sqlalchemy import Float
 from sqlalchemy import func
 from sqlalchemy import Identity
+from sqlalchemy import Index
 from sqlalchemy import inspect
 from sqlalchemy import Integer
 from sqlalchemy import MetaData
@@ -15,6 +16,7 @@
 from sqlalchemy import TIMESTAMP
 from sqlalchemy.dialects.mysql import VARCHAR
 
+from alembic import autogenerate
 from alembic import op
 from alembic import util
 from alembic.autogenerate import api
@@ -24,6 +26,7 @@
 from alembic.testing import assert_raises_message
 from alembic.testing import combinations
 from alembic.testing import config
+from alembic.testing import eq_ignore_whitespace
 from alembic.testing.env import clear_staging_env
 from alembic.testing.env import staging_env
 from alembic.testing.fixtures import AlterColRoundTripFixture
@@ -692,3 +695,79 @@ def test_compare_boolean_same(self):
 
     def test_compare_boolean_diff(self):
         self._compare_default_roundtrip(Boolean(), "1", "0")
+
+
+class MySQLAutogenRenderTest(TestBase):
+    def setUp(self):
+        ctx_opts = {
+            "sqlalchemy_module_prefix": "sa.",
+            "alembic_module_prefix": "op.",
+            "target_metadata": MetaData(),
+        }
+        context = MigrationContext.configure(
+            dialect_name="mysql", opts=ctx_opts
+        )
+
+        self.autogen_context = api.AutogenContext(context)
+
+    def test_render_add_index_expr_binary(self):
+        m = MetaData()
+        t = Table(
+            "t",
+            m,
+            Column("x", Integer, primary_key=True),
+            Column("y", Integer),
+        )
+        idx = Index("foo_idx", t.c.x > 5)
+
+        eq_ignore_whitespace(
+            autogenerate.render_op_text(
+                self.autogen_context, ops.CreateIndexOp.from_index(idx)
+            ),
+            "op.create_index('foo_idx', 't', "
+            "[sa.literal_column('(x > 5)')], unique=False)",
+        )
+
+    def test_render_add_index_expr_unary(self):
+        m = MetaData()
+        t = Table(
+            "t",
+            m,
+            Column("x", Integer, primary_key=True),
+            Column("y", Integer),
+        )
+        idx1 = Index("foo_idx", -t.c.x)
+        idx2 = Index("foo_idx", t.c.x.desc())
+
+        eq_ignore_whitespace(
+            autogenerate.render_op_text(
+                self.autogen_context, ops.CreateIndexOp.from_index(idx1)
+            ),
+            "op.create_index('foo_idx', 't', "
+            "[sa.literal_column('(-x)')], unique=False)",
+        )
+        eq_ignore_whitespace(
+            autogenerate.render_op_text(
+                self.autogen_context, ops.CreateIndexOp.from_index(idx2)
+            ),
+            "op.create_index('foo_idx', 't', "
+            "[sa.literal_column('x DESC')], unique=False)",
+        )
+
+    def test_render_add_index_expr_func(self):
+        m = MetaData()
+        t = Table(
+            "t",
+            m,
+            Column("x", Integer, primary_key=True),
+            Column("y", Integer, nullable=True),
+        )
+        idx = Index("foo_idx", t.c.x, func.coalesce(t.c.y, 0))
+
+        eq_ignore_whitespace(
+            autogenerate.render_op_text(
+                self.autogen_context, ops.CreateIndexOp.from_index(idx)
+            ),
+            "op.create_index('foo_idx', 't', "
+            "['x', sa.literal_column('(coalesce(y, 0))')], unique=False)",
+        )