ModelsMigrationsSync:Override compare_server_default

This test showed an error with alembic 0.6.6:
For MySQL Integer server_default value is compared incorrectly.
For example, when model has DefaultClause('0', for_update=False)
alembic finds it different from server variant "'0'".
For PostgreSQL Sting server_default value is compared incorrectly.
For example, when model has DefaultClause('', for_update=False)
alembic finds it different from server variant "''::character varying".

Such error appear in implementation of ModelsMigrationsSync to Neutron
https://review.openstack.org/76520.

Change-Id: I9730da1c0790a88f81570c51c36cc0cbe0651a5a

2 files changed, 34 insertions, 4 deletions

diff --git a/oslo/db/sqlalchemy/test_migrations.py b/oslo/db/sqlalchemy/test_migrations.py
index 36f8ae1..bdd7c6e 100644
--- a/oslo/db/sqlalchemy/test_migrations.py
+++ b/oslo/db/sqlalchemy/test_migrations.py
@@ -32,6 +32,7 @@ import sqlalchemy.types as types
 
 from oslo.db._i18n import _LE
 from oslo.db import exception as exc
+from oslo.db.sqlalchemy import utils
 
 LOG = logging.getLogger(__name__)
 
@@ -346,19 +347,34 @@ class ModelsMigrationsSync(object):
         :param rendered_meta_def: rendered column default value (from model)
 
         """
+        return self._compare_server_default(ctxt.bind, meta_col, insp_def,
+                                            meta_def)
 
-        if (ctxt.dialect.name == 'mysql' and
-                issubclass(type(meta_col.type), sqlalchemy.Boolean)):
+    @utils.DialectFunctionDispatcher.dispatch_for_dialect("*")
+    def _compare_server_default(bind, meta_col, insp_def, meta_def):
+        pass
 
+    @_compare_server_default.dispatch_for('mysql')
+    def _compare_server_default(bind, meta_col, insp_def, meta_def):
+        if isinstance(meta_col.type, sqlalchemy.Boolean):
             if meta_def is None or insp_def is None:
                 return meta_def != insp_def
-
             return not (
                 isinstance(meta_def.arg, expr.True_) and insp_def == "'1'" or
                 isinstance(meta_def.arg, expr.False_) and insp_def == "'0'"
             )
 
-        return None  # tells alembic to use the default comparison method
+        if isinstance(meta_col.type, sqlalchemy.Integer):
+            if meta_def is None or insp_def is None:
+                return meta_def != insp_def
+            return meta_def.arg != insp_def.split("'")[1]
+
+    @_compare_server_default.dispatch_for('postgresql')
+    def _compare_server_default(bind, meta_col, insp_def, meta_def):
+        if isinstance(meta_col.type, sqlalchemy.String):
+            if meta_def is None or insp_def is None:
+                return meta_def != insp_def
+            return insp_def != "'%s'::character varying" % meta_def.arg
 
     def _cleanup(self):
         engine = self.get_engine()
diff --git a/tests/sqlalchemy/test_migrations.py b/tests/sqlalchemy/test_migrations.py
index bb50e4e..ed93c3d 100644
--- a/tests/sqlalchemy/test_migrations.py
+++ b/tests/sqlalchemy/test_migrations.py
@@ -188,6 +188,9 @@ class ModelsMigrationSyncMixin(test.BaseTestCase):
                       server_default=sa.sql.expression.true()),
             sa.Column('bool_wo_default', sa.Boolean),
             sa.Column('bar', sa.Numeric(10, 5)),
+            sa.Column('defaulttest', sa.Integer, server_default='5'),
+            sa.Column('defaulttest2', sa.String(8), server_default=''),
+            sa.Column('defaulttest3', sa.String(5), server_default="test"),
             sa.UniqueConstraint('spam', 'eggs', name='uniq_cons'),
         )
 
@@ -205,6 +208,12 @@ class ModelsMigrationSyncMixin(test.BaseTestCase):
             foo = sa.Column('foo', sa.Boolean,
                             server_default=sa.sql.expression.true())
             bool_wo_default = sa.Column('bool_wo_default', sa.Boolean)
+            defaulttest = sa.Column('defaulttest',
+                                    sa.Integer, server_default='5')
+            defaulttest2 = sa.Column('defaulttest2', sa.String(8),
+                                     server_default='')
+            defaulttest3 = sa.Column('defaulttest3', sa.String(5),
+                                     server_default="test")
             bar = sa.Column('bar', sa.Numeric(10, 5))
 
         class ModelThatShouldNotBeCompared(BASE):
@@ -236,6 +245,9 @@ class ModelsMigrationSyncMixin(test.BaseTestCase):
                       server_default=sa.sql.expression.false()),
             sa.Column('bool_wo_default', sa.Boolean, unique=True),
             sa.Column('bar', sa.BigInteger),
+            sa.Column('defaulttest', sa.Integer, server_default='7'),
+            sa.Column('defaulttest2', sa.String(8), server_default=''),
+            sa.Column('defaulttest3', sa.String(5), server_default="fake"),
             sa.UniqueConstraint('spam', 'foo', name='uniq_cons'),
         )
 
@@ -253,6 +265,8 @@ class ModelsMigrationSyncMixin(test.BaseTestCase):
         self.assertIn('foo', msg)
         self.assertIn('bar', msg)
         self.assertIn('bool_wo_default', msg)
+        self.assertIn('defaulttest', msg)
+        self.assertIn('defaulttest3', msg)
 
 
 class ModelsMigrationsSyncMysql(ModelsMigrationSyncMixin,