From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Thu, 3 Sep 2020 18:14:39 +0000 (-0400)
Subject: Deprecate duplicated column names in Table definition
X-Git-Tag: rel_1_4_0b1~38
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9e82f32f274e649b04740c819d21ba232c89cfff;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Deprecate duplicated column names in Table definition

The :class:`_schema.Table` class now raises a deprecation warning
when columns with the same name are defined. To replace a column a new
parameter :paramref:`_schema.Table.append_column.replace_existing` was
added to the :meth:`_schema.Table.append_column` method.

The :meth:`_expression.ColumnCollection.contains_column` will now
raises an error when called with a string, suggesting the caller
to use ``in`` instead.

Co-authored-by: Federico Caselli <cfederico87@gmail.com>
Change-Id: I1d58c8ebe081079cb669e7ead60886ffc1b1a7f5
---

diff --git a/doc/build/changelog/unreleased_14/table_no_dupe_cols.rst b/doc/build/changelog/unreleased_14/table_no_dupe_cols.rst
new file mode 100644
index 0000000000..7e1e488acc
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/table_no_dupe_cols.rst
@@ -0,0 +1,12 @@
+.. change::
+    :tags: change, sql
+    :tickets: 5526
+
+    The :class:`_schema.Table` class now raises a deprecation warning
+    when columns with the same name are defined. To replace a column a new
+    parameter :paramref:`_schema.Table.append_column.replace_existing` was
+    added to the :meth:`_schema.Table.append_column` method.
+
+    The :meth:`_expression.ColumnCollection.contains_column` will now
+    raises an error when called with a string, suggesting the caller
+    to use ``in`` instead.
diff --git a/examples/versioned_history/history_meta.py b/examples/versioned_history/history_meta.py
index f2b3f8118d..7d13f2d745 100644
--- a/examples/versioned_history/history_meta.py
+++ b/examples/versioned_history/history_meta.py
@@ -163,7 +163,8 @@ def _history_mapper(local_mapper):
 
     if not super_history_mapper:
         local_mapper.local_table.append_column(
-            Column("version", Integer, default=1, nullable=False)
+            Column("version", Integer, default=1, nullable=False),
+            replace_existing=True,
         )
         local_mapper.add_property(
             "version", local_mapper.local_table.c.version
diff --git a/lib/sqlalchemy/dialects/postgresql/ext.py b/lib/sqlalchemy/dialects/postgresql/ext.py
index 7acab0a0a7..c139fe94fc 100644
--- a/lib/sqlalchemy/dialects/postgresql/ext.py
+++ b/lib/sqlalchemy/dialects/postgresql/ext.py
@@ -205,7 +205,7 @@ class ExcludeConstraint(ColumnCollectionConstraint):
         if where is not None:
             self.where = coercions.expect(roles.StatementOptionRole, where)
 
-    def _set_parent(self, table):
+    def _set_parent(self, table, **kw):
         super(ExcludeConstraint, self)._set_parent(table)
 
         self._render_exprs = [
diff --git a/lib/sqlalchemy/engine/reflection.py b/lib/sqlalchemy/engine/reflection.py
index 92a85df2e6..6af74d0527 100644
--- a/lib/sqlalchemy/engine/reflection.py
+++ b/lib/sqlalchemy/engine/reflection.py
@@ -898,7 +898,7 @@ class Inspector(object):
 
         if col.key in table.primary_key:
             col.primary_key = True
-        table.append_column(col)
+        table.append_column(col, replace_existing=True)
 
     def _reflect_col_sequence(self, col_d, colargs):
         if "sequence" in col_d:
diff --git a/lib/sqlalchemy/orm/decl_base.py b/lib/sqlalchemy/orm/decl_base.py
index 0e89e729fb..35da39c9f1 100644
--- a/lib/sqlalchemy/orm/decl_base.py
+++ b/lib/sqlalchemy/orm/decl_base.py
@@ -909,13 +909,13 @@ def _add_attribute(cls, key, value):
     if "__mapper__" in cls.__dict__:
         if isinstance(value, Column):
             _undefer_column_name(key, value)
-            cls.__table__.append_column(value)
+            cls.__table__.append_column(value, replace_existing=True)
             cls.__mapper__.add_property(key, value)
         elif isinstance(value, ColumnProperty):
             for col in value.columns:
                 if isinstance(col, Column) and col.table is None:
                     _undefer_column_name(key, col)
-                    cls.__table__.append_column(col)
+                    cls.__table__.append_column(col, replace_existing=True)
             cls.__mapper__.add_property(key, value)
         elif isinstance(value, MapperProperty):
             cls.__mapper__.add_property(key, value)
diff --git a/lib/sqlalchemy/sql/base.py b/lib/sqlalchemy/sql/base.py
index f9b5ce7e19..f912163bc6 100644
--- a/lib/sqlalchemy/sql/base.py
+++ b/lib/sqlalchemy/sql/base.py
@@ -956,12 +956,12 @@ class SchemaEventTarget(object):
 
     """
 
-    def _set_parent(self, parent):
+    def _set_parent(self, parent, **kw):
         """Associate with this SchemaEvent's parent object."""
 
-    def _set_parent_with_dispatch(self, parent):
+    def _set_parent_with_dispatch(self, parent, **kw):
         self.dispatch.before_parent_attach(self, parent)
-        self._set_parent(parent)
+        self._set_parent(parent, **kw)
         self.dispatch.after_parent_attach(self, parent)
 
 
@@ -1185,7 +1185,16 @@ class ColumnCollection(object):
         )
 
     def contains_column(self, col):
-        return col in self._colset
+        """Checks if a column object exists in this collection"""
+        if col not in self._colset:
+            if isinstance(col, util.string_types):
+                raise exc.ArgumentError(
+                    "contains_column cannot be used with string arguments. "
+                    "Use ``col_name in table.c`` instead."
+                )
+            return False
+        else:
+            return True
 
     def as_immutable(self):
         return ImmutableColumnCollection(self)
@@ -1302,6 +1311,7 @@ class DedupeColumnCollection(ColumnCollection):
     """
 
     def add(self, column, key=None):
+
         if key is not None and column.key != key:
             raise exc.ArgumentError(
                 "DedupeColumnCollection requires columns be under "
diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py
index 50d7d1f5b8..f1cfaaef4f 100644
--- a/lib/sqlalchemy/sql/schema.py
+++ b/lib/sqlalchemy/sql/schema.py
@@ -100,9 +100,8 @@ class SchemaItem(SchemaEventTarget, visitors.Visitable):
 
     __visit_name__ = "schema_item"
 
-    def _init_items(self, *args):
+    def _init_items(self, *args, **kw):
         """Initialize the list of child items for this SchemaItem."""
-
         for item in args:
             if item is not None:
                 try:
@@ -116,7 +115,7 @@ class SchemaItem(SchemaEventTarget, visitors.Visitable):
                         replace_context=err,
                     )
                 else:
-                    spwd(self)
+                    spwd(self, **kw)
 
     def __repr__(self):
         return util.generic_repr(self, omit_kwarg=["info"])
@@ -202,14 +201,14 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
         :class:`.PrimaryKeyConstraint`, and
         :class:`_schema.ForeignKeyConstraint`.
 
-    :param autoload: Defaults to False, unless
+    :param autoload: Defaults to ``False``, unless
         :paramref:`_schema.Table.autoload_with`
-        is set in which case it defaults to True; :class:`_schema.Column`
-        objects
+        is set in which case it defaults to ``True``;
+        :class:`_schema.Column` objects
         for this table should be reflected from the database, possibly
-        augmenting or replacing existing :class:`_schema.Column`
-        objects that were
-        explicitly specified.
+        augmenting objects that were explicitly specified.
+        :class:`_schema.Column` and other objects explicitly set on the
+        table will replace corresponding reflected objects.
 
         .. deprecated:: 1.4
 
@@ -501,8 +500,8 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
             schema = metadata.schema
         elif schema is BLANK_SCHEMA:
             schema = None
-        keep_existing = kw.pop("keep_existing", False)
-        extend_existing = kw.pop("extend_existing", False)
+        keep_existing = kw.get("keep_existing", False)
+        extend_existing = kw.get("extend_existing", False)
 
         if keep_existing and extend_existing:
             msg = "keep_existing and extend_existing are mutually exclusive."
@@ -533,7 +532,7 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
                 table._init(name, metadata, *args, **kw)
                 table.dispatch.after_parent_attach(table, metadata)
                 return table
-            except:
+            except Exception:
                 with util.safe_reraise():
                     metadata._remove_table(name, schema)
 
@@ -565,7 +564,6 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
 
         self.indexes = set()
         self.constraints = set()
-        self._columns = DedupeColumnCollection()
         PrimaryKeyConstraint(
             _implicit_generated=True
         )._set_parent_with_dispatch(self)
@@ -580,6 +578,8 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
         autoload = kwargs.pop("autoload", autoload_with is not None)
         # this argument is only used with _init_existing()
         kwargs.pop("autoload_replace", True)
+        keep_existing = kwargs.pop("keep_existing", False)
+        extend_existing = kwargs.pop("extend_existing", False)
         _extend_on = kwargs.pop("_extend_on", None)
 
         resolve_fks = kwargs.pop("resolve_fks", True)
@@ -614,7 +614,11 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
 
         # initialize all the column, etc. objects.  done after reflection to
         # allow user-overrides
-        self._init_items(*args)
+
+        self._init_items(
+            *args,
+            allow_replacements=extend_existing or keep_existing or autoload
+        )
 
     def _autoload(
         self,
@@ -670,6 +674,9 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
         autoload_replace = kwargs.pop("autoload_replace", True)
         schema = kwargs.pop("schema", None)
         _extend_on = kwargs.pop("_extend_on", None)
+        # these arguments are only used with _init()
+        kwargs.pop("extend_existing", False)
+        kwargs.pop("keep_existing", False)
 
         if schema and schema != self.schema:
             raise exc.ArgumentError(
@@ -776,7 +783,7 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
         """
         self._extra_dependencies.add(table)
 
-    def append_column(self, column):
+    def append_column(self, column, replace_existing=False):
         """Append a :class:`_schema.Column` to this :class:`_schema.Table`.
 
         The "key" of the newly added :class:`_schema.Column`, i.e. the
@@ -794,9 +801,17 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
         emitted for an already-existing table that doesn't contain
         the newly added column.
 
+        :param replace_existing: When ``True``, allows replacing existing
+            columns. When ``False``, the default, an warning will be raised
+            if a column with the same ``.key`` already exists. A future
+            version of sqlalchemy will instead rise a warning.
+
+            .. versionadded:: 1.4.0
         """
 
-        column._set_parent_with_dispatch(self)
+        column._set_parent_with_dispatch(
+            self, allow_replacements=replace_existing
+        )
 
     def append_constraint(self, constraint):
         """Append a :class:`_schema.Constraint` to this
@@ -819,7 +834,7 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
 
         constraint._set_parent_with_dispatch(self)
 
-    def _set_parent(self, metadata):
+    def _set_parent(self, metadata, **kw):
         metadata._add_table(self.name, self.schema, self)
         self.metadata = metadata
 
@@ -1495,7 +1510,7 @@ class Column(DialectKWArgs, SchemaItem, ColumnClause):
             + ["%s=%s" % (k, repr(getattr(self, k))) for k in kwarg]
         )
 
-    def _set_parent(self, table):
+    def _set_parent(self, table, allow_replacements=True):
         if not self.name:
             raise exc.ArgumentError(
                 "Column must be constructed with a non-blank name or "
@@ -1517,6 +1532,15 @@ class Column(DialectKWArgs, SchemaItem, ColumnClause):
         if self.key in table._columns:
             col = table._columns.get(self.key)
             if col is not self:
+                if not allow_replacements:
+                    util.warn_deprecated(
+                        "A column with name '%s' is already present "
+                        "in table '%s'. Please use method "
+                        ":meth:`_schema.Table.append_column` with the "
+                        "parameter ``replace_existing=True`` to replace an "
+                        "existing column." % (self.key, table.name),
+                        "1.4",
+                    )
                 for fk in col.foreign_keys:
                     table.foreign_keys.remove(fk)
                     if fk.constraint in table.constraints:
@@ -2129,7 +2153,7 @@ class ForeignKey(DialectKWArgs, SchemaItem):
             _column = self._colspec
             return _column
 
-    def _set_parent(self, column):
+    def _set_parent(self, column, **kw):
         if self.parent is not None and self.parent is not column:
             raise exc.InvalidRequestError(
                 "This ForeignKey already has a parent !"
@@ -2204,7 +2228,7 @@ class DefaultGenerator(SchemaItem):
     def __init__(self, for_update=False):
         self.for_update = for_update
 
-    def _set_parent(self, column):
+    def _set_parent(self, column, **kw):
         self.column = column
         if self.for_update:
             self.column.onupdate = self
@@ -2653,7 +2677,7 @@ class Sequence(IdentityOptions, roles.StatementRole, DefaultGenerator):
             self, bind=self.bind
         )
 
-    def _set_parent(self, column):
+    def _set_parent(self, column, **kw):
         super(Sequence, self)._set_parent(column)
         column._on_table_attach(self._set_table)
 
@@ -2736,7 +2760,7 @@ class FetchedValue(SchemaEventTarget):
         n.for_update = for_update
         return n
 
-    def _set_parent(self, column):
+    def _set_parent(self, column, **kw):
         self.column = column
         if self.for_update:
             self.column.server_onupdate = self
@@ -2862,7 +2886,7 @@ class Constraint(DialectKWArgs, SchemaItem):
             "mean to call table.append_constraint(constraint) ?"
         )
 
-    def _set_parent(self, parent):
+    def _set_parent(self, parent, **kw):
         self.parent = parent
         parent.constraints.add(self)
 
@@ -2965,7 +2989,7 @@ class ColumnCollectionMixin(object):
             for col in self._pending_colargs
         ]
 
-    def _set_parent(self, table):
+    def _set_parent(self, table, **kw):
         for col in self._col_expressions(table):
             if col is not None:
                 self.columns.add(col)
@@ -3007,7 +3031,7 @@ class ColumnCollectionConstraint(ColumnCollectionMixin, Constraint):
 
     """
 
-    def _set_parent(self, table):
+    def _set_parent(self, table, **kw):
         Constraint._set_parent(self, table)
         ColumnCollectionMixin._set_parent(self, table)
 
@@ -3394,7 +3418,7 @@ class ForeignKeyConstraint(ColumnCollectionConstraint):
     def _col_description(self):
         return ", ".join(self.column_keys)
 
-    def _set_parent(self, table):
+    def _set_parent(self, table, **kw):
         Constraint._set_parent(self, table)
 
         try:
@@ -3522,7 +3546,7 @@ class PrimaryKeyConstraint(ColumnCollectionConstraint):
         self._implicit_generated = kw.pop("_implicit_generated", False)
         super(PrimaryKeyConstraint, self).__init__(*columns, **kw)
 
-    def _set_parent(self, table):
+    def _set_parent(self, table, **kw):
         super(PrimaryKeyConstraint, self)._set_parent(table)
 
         if table.primary_key is not self:
@@ -3809,7 +3833,7 @@ class Index(DialectKWArgs, ColumnCollectionMixin, SchemaItem):
         if table is not None:
             self._set_parent(table)
 
-    def _set_parent(self, table):
+    def _set_parent(self, table, **kw):
         ColumnCollectionMixin._set_parent(self, table)
 
         if self.table is not None and table is not self.table:
@@ -4607,7 +4631,7 @@ class Computed(FetchedValue, SchemaItem):
         self.persisted = persisted
         self.column = None
 
-    def _set_parent(self, parent):
+    def _set_parent(self, parent, **kw):
         if not isinstance(
             parent.server_default, (type(None), Computed)
         ) or not isinstance(parent.server_onupdate, (type(None), Computed)):
@@ -4734,7 +4758,7 @@ class Identity(IdentityOptions, FetchedValue, SchemaItem):
         self.on_null = on_null
         self.column = None
 
-    def _set_parent(self, parent):
+    def _set_parent(self, parent, **kw):
         if not isinstance(
             parent.server_default, (type(None), Identity)
         ) or not isinstance(parent.server_onupdate, type(None)):
diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py
index 951f50ae1b..eebf3b96ea 100644
--- a/lib/sqlalchemy/sql/selectable.py
+++ b/lib/sqlalchemy/sql/selectable.py
@@ -2167,7 +2167,7 @@ class TableClause(roles.DMLTableRole, Immutable, FromClause):
         else:
             return self.name.encode("ascii", "backslashreplace")
 
-    def append_column(self, c):
+    def append_column(self, c, **kw):
         existing = c.table
         if existing is not None and existing is not self:
             raise exc.ArgumentError(
diff --git a/test/base/test_utils.py b/test/base/test_utils.py
index 843d36b71d..876cb7a440 100644
--- a/test/base/test_utils.py
+++ b/test/base/test_utils.py
@@ -460,6 +460,23 @@ class ColumnCollectionCommon(testing.AssertsCompiledSQL):
         is_true(cc.contains_column(c1))
         is_false(cc.contains_column(c3))
 
+    def test_contains_column_not_column(self):
+        c1, c2, c3 = sql.column("c1"), sql.column("c2"), sql.column("c3")
+        cc = self._column_collection(columns=[("c1", c1), ("c2", c2)])
+
+        is_false(cc.contains_column(c3 == 2))
+
+        with testing.expect_raises_message(
+            exc.ArgumentError,
+            "contains_column cannot be used with string arguments",
+        ):
+            cc.contains_column("c1")
+        with testing.expect_raises_message(
+            exc.ArgumentError,
+            "contains_column cannot be used with string arguments",
+        ):
+            cc.contains_column("foo")
+
     def test_in(self):
         col1 = sql.column("col1")
         cc = self._column_collection(
diff --git a/test/sql/test_defaults.py b/test/sql/test_defaults.py
index 2750568d87..4a6ebd0c83 100644
--- a/test/sql/test_defaults.py
+++ b/test/sql/test_defaults.py
@@ -293,13 +293,6 @@ class DefaultObjectTest(fixtures.TestBase):
             ),
             Column("col3", Integer, Sequence("foo"), server_default="x"),
             Column("col4", Integer, ColumnDefault("x"), DefaultClause("y")),
-            Column(
-                "col4",
-                Integer,
-                ColumnDefault("x"),
-                DefaultClause("y"),
-                DefaultClause("y", for_update=True),
-            ),
             Column(
                 "col5",
                 Integer,
@@ -326,6 +319,16 @@ class DefaultObjectTest(fixtures.TestBase):
                 onupdate="z",
             ),
         )
+        tbl.append_column(
+            Column(
+                "col4",
+                Integer,
+                ColumnDefault("x"),
+                DefaultClause("y"),
+                DefaultClause("y", for_update=True),
+            ),
+            replace_existing=True,
+        )
         has_(tbl, "col1", "default")
         has_(tbl, "col2", "default", "server_default")
         has_(tbl, "col3", "default", "server_default")
diff --git a/test/sql/test_metadata.py b/test/sql/test_metadata.py
index d378c8184d..980e2886ab 100644
--- a/test/sql/test_metadata.py
+++ b/test/sql/test_metadata.py
@@ -1683,6 +1683,50 @@ class TableTest(fixtures.TestBase, AssertsCompiledSQL):
         is_(t2.c.contains_column(z), True)
         is_(t2.c.contains_column(g), False)
 
+    def test_table_ctor_duplicated_column_name(self):
+        def go():
+            return Table(
+                "t",
+                MetaData(),
+                Column("a", Integer),
+                Column("col", Integer),
+                Column("col", String),
+            )
+
+        with testing.expect_deprecated(
+            "A column with name 'col' is already present in table 't'",
+        ):
+            t = go()
+        is_true(isinstance(t.c.col.type, String))
+        # when it will raise
+        # with testing.expect_raises_message(
+        #     exc.ArgumentError,
+        #     "A column with name 'col' is already present in table 't'",
+        # ):
+        #     go()
+
+    def test_append_column_existing_name(self):
+        t = Table("t", MetaData(), Column("col", Integer))
+
+        with testing.expect_deprecated(
+            "A column with name 'col' is already present in table 't'",
+        ):
+            t.append_column(Column("col", String))
+        is_true(isinstance(t.c.col.type, String))
+        # when it will raise
+        # col = t.c.col
+        # with testing.expect_raises_message(
+        #     exc.ArgumentError,
+        #     "A column with name 'col' is already present in table 't'",
+        # ):
+        #     t.append_column(Column("col", String))
+        # is_true(t.c.col is col)
+
+    def test_append_column_replace_existing(self):
+        t = Table("t", MetaData(), Column("col", Integer))
+        t.append_column(Column("col", String), replace_existing=True)
+        is_true(isinstance(t.c.col.type, String))
+
     def test_autoincrement_replace(self):
         m = MetaData()
 
@@ -2392,19 +2436,21 @@ class UseExistingTest(fixtures.TablesTest):
             Column("name", String(30)),
         )
 
-    def _useexisting_fixture(self):
+    @testing.fixture
+    def existing_meta(self):
         meta2 = MetaData(testing.db)
         Table("users", meta2, autoload=True)
         return meta2
 
-    def _notexisting_fixture(self):
+    @testing.fixture
+    def empty_meta(self):
         return MetaData(testing.db)
 
-    def test_exception_no_flags(self):
-        meta2 = self._useexisting_fixture()
-
+    def test_exception_no_flags(self, existing_meta):
         def go():
-            Table("users", meta2, Column("name", Unicode), autoload=True)
+            Table(
+                "users", existing_meta, Column("name", Unicode), autoload=True
+            )
 
         assert_raises_message(
             exc.InvalidRequestError,
@@ -2412,22 +2458,20 @@ class UseExistingTest(fixtures.TablesTest):
             go,
         )
 
-    def test_keep_plus_existing_raises(self):
-        meta2 = self._useexisting_fixture()
+    def test_keep_plus_existing_raises(self, existing_meta):
         assert_raises(
             exc.ArgumentError,
             Table,
             "users",
-            meta2,
+            existing_meta,
             keep_existing=True,
             extend_existing=True,
         )
 
-    def test_keep_existing_no_dupe_constraints(self):
-        meta2 = self._notexisting_fixture()
+    def test_keep_existing_no_dupe_constraints(self, empty_meta):
         users = Table(
             "users",
-            meta2,
+            empty_meta,
             Column("id", Integer),
             Column("name", Unicode),
             UniqueConstraint("name"),
@@ -2439,7 +2483,7 @@ class UseExistingTest(fixtures.TablesTest):
 
         u2 = Table(
             "users",
-            meta2,
+            empty_meta,
             Column("id", Integer),
             Column("name", Unicode),
             UniqueConstraint("name"),
@@ -2447,11 +2491,10 @@ class UseExistingTest(fixtures.TablesTest):
         )
         eq_(len(u2.constraints), 2)
 
-    def test_extend_existing_dupes_constraints(self):
-        meta2 = self._notexisting_fixture()
+    def test_extend_existing_dupes_constraints(self, empty_meta):
         users = Table(
             "users",
-            meta2,
+            empty_meta,
             Column("id", Integer),
             Column("name", Unicode),
             UniqueConstraint("name"),
@@ -2463,7 +2506,7 @@ class UseExistingTest(fixtures.TablesTest):
 
         u2 = Table(
             "users",
-            meta2,
+            empty_meta,
             Column("id", Integer),
             Column("name", Unicode),
             UniqueConstraint("name"),
@@ -2472,40 +2515,46 @@ class UseExistingTest(fixtures.TablesTest):
         # constraint got duped
         eq_(len(u2.constraints), 3)
 
-    def test_keep_existing_coltype(self):
-        meta2 = self._useexisting_fixture()
+    def test_autoload_replace_column(self, empty_meta):
+        users = Table(
+            "users", empty_meta, Column("name", Unicode), autoload=True
+        )
+        assert isinstance(users.c.name.type, Unicode)
+
+    def test_keep_existing_coltype(self, existing_meta):
         users = Table(
             "users",
-            meta2,
+            existing_meta,
             Column("name", Unicode),
             autoload=True,
             keep_existing=True,
         )
         assert not isinstance(users.c.name.type, Unicode)
 
-    def test_keep_existing_quote(self):
-        meta2 = self._useexisting_fixture()
+    def test_keep_existing_quote(self, existing_meta):
         users = Table(
-            "users", meta2, quote=True, autoload=True, keep_existing=True
+            "users",
+            existing_meta,
+            quote=True,
+            autoload=True,
+            keep_existing=True,
         )
         assert not users.name.quote
 
-    def test_keep_existing_add_column(self):
-        meta2 = self._useexisting_fixture()
+    def test_keep_existing_add_column(self, existing_meta):
         users = Table(
             "users",
-            meta2,
+            existing_meta,
             Column("foo", Integer),
             autoload=True,
             keep_existing=True,
         )
         assert "foo" not in users.c
 
-    def test_keep_existing_coltype_no_orig(self):
-        meta2 = self._notexisting_fixture()
+    def test_keep_existing_coltype_no_orig(self, empty_meta):
         users = Table(
             "users",
-            meta2,
+            empty_meta,
             Column("name", Unicode),
             autoload=True,
             keep_existing=True,
@@ -2516,83 +2565,74 @@ class UseExistingTest(fixtures.TablesTest):
         lambda: testing.db.dialect.requires_name_normalize,
         "test depends on lowercase as case insensitive",
     )
-    def test_keep_existing_quote_no_orig(self):
-        meta2 = self._notexisting_fixture()
+    def test_keep_existing_quote_no_orig(self, empty_meta):
         users = Table(
-            "users", meta2, quote=True, autoload=True, keep_existing=True
+            "users", empty_meta, quote=True, autoload=True, keep_existing=True
         )
         assert users.name.quote
 
-    def test_keep_existing_add_column_no_orig(self):
-        meta2 = self._notexisting_fixture()
+    def test_keep_existing_add_column_no_orig(self, empty_meta):
         users = Table(
             "users",
-            meta2,
+            empty_meta,
             Column("foo", Integer),
             autoload=True,
             keep_existing=True,
         )
         assert "foo" in users.c
 
-    def test_keep_existing_coltype_no_reflection(self):
-        meta2 = self._useexisting_fixture()
+    def test_keep_existing_coltype_no_reflection(self, existing_meta):
         users = Table(
-            "users", meta2, Column("name", Unicode), keep_existing=True
+            "users", existing_meta, Column("name", Unicode), keep_existing=True
         )
         assert not isinstance(users.c.name.type, Unicode)
 
-    def test_keep_existing_quote_no_reflection(self):
-        meta2 = self._useexisting_fixture()
-        users = Table("users", meta2, quote=True, keep_existing=True)
+    def test_keep_existing_quote_no_reflection(self, existing_meta):
+        users = Table("users", existing_meta, quote=True, keep_existing=True)
         assert not users.name.quote
 
-    def test_keep_existing_add_column_no_reflection(self):
-        meta2 = self._useexisting_fixture()
+    def test_keep_existing_add_column_no_reflection(self, existing_meta):
         users = Table(
-            "users", meta2, Column("foo", Integer), keep_existing=True
+            "users", existing_meta, Column("foo", Integer), keep_existing=True
         )
         assert "foo" not in users.c
 
-    def test_extend_existing_coltype(self):
-        meta2 = self._useexisting_fixture()
+    def test_extend_existing_coltype(self, existing_meta):
         users = Table(
             "users",
-            meta2,
+            existing_meta,
             Column("name", Unicode),
             autoload=True,
             extend_existing=True,
         )
         assert isinstance(users.c.name.type, Unicode)
 
-    def test_extend_existing_quote(self):
-        meta2 = self._useexisting_fixture()
+    def test_extend_existing_quote(self, existing_meta):
         assert_raises_message(
             tsa.exc.ArgumentError,
             "Can't redefine 'quote' or 'quote_schema' arguments",
             Table,
             "users",
-            meta2,
+            existing_meta,
             quote=True,
             autoload=True,
             extend_existing=True,
         )
 
-    def test_extend_existing_add_column(self):
-        meta2 = self._useexisting_fixture()
+    def test_extend_existing_add_column(self, existing_meta):
         users = Table(
             "users",
-            meta2,
+            existing_meta,
             Column("foo", Integer),
             autoload=True,
             extend_existing=True,
         )
         assert "foo" in users.c
 
-    def test_extend_existing_coltype_no_orig(self):
-        meta2 = self._notexisting_fixture()
+    def test_extend_existing_coltype_no_orig(self, empty_meta):
         users = Table(
             "users",
-            meta2,
+            empty_meta,
             Column("name", Unicode),
             autoload=True,
             extend_existing=True,
@@ -2603,47 +2643,52 @@ class UseExistingTest(fixtures.TablesTest):
         lambda: testing.db.dialect.requires_name_normalize,
         "test depends on lowercase as case insensitive",
     )
-    def test_extend_existing_quote_no_orig(self):
-        meta2 = self._notexisting_fixture()
+    def test_extend_existing_quote_no_orig(self, empty_meta):
         users = Table(
-            "users", meta2, quote=True, autoload=True, extend_existing=True
+            "users",
+            empty_meta,
+            quote=True,
+            autoload=True,
+            extend_existing=True,
         )
         assert users.name.quote
 
-    def test_extend_existing_add_column_no_orig(self):
-        meta2 = self._notexisting_fixture()
+    def test_extend_existing_add_column_no_orig(self, empty_meta):
         users = Table(
             "users",
-            meta2,
+            empty_meta,
             Column("foo", Integer),
             autoload=True,
             extend_existing=True,
         )
         assert "foo" in users.c
 
-    def test_extend_existing_coltype_no_reflection(self):
-        meta2 = self._useexisting_fixture()
+    def test_extend_existing_coltype_no_reflection(self, existing_meta):
         users = Table(
-            "users", meta2, Column("name", Unicode), extend_existing=True
+            "users",
+            existing_meta,
+            Column("name", Unicode),
+            extend_existing=True,
         )
         assert isinstance(users.c.name.type, Unicode)
 
-    def test_extend_existing_quote_no_reflection(self):
-        meta2 = self._useexisting_fixture()
+    def test_extend_existing_quote_no_reflection(self, existing_meta):
         assert_raises_message(
             tsa.exc.ArgumentError,
             "Can't redefine 'quote' or 'quote_schema' arguments",
             Table,
             "users",
-            meta2,
+            existing_meta,
             quote=True,
             extend_existing=True,
         )
 
-    def test_extend_existing_add_column_no_reflection(self):
-        meta2 = self._useexisting_fixture()
+    def test_extend_existing_add_column_no_reflection(self, existing_meta):
         users = Table(
-            "users", meta2, Column("foo", Integer), extend_existing=True
+            "users",
+            existing_meta,
+            Column("foo", Integer),
+            extend_existing=True,
         )
         assert "foo" in users.c