Add _extend_on deduplicating set for metadata.reflect()

The "extend_existing" option of :class:`.Table` reflection would
cause indexes and constraints to be doubled up in the case that the parameter
were used with :meth:`.MetaData.reflect` (as the automap extension does)
due to tables being reflected both within the foreign key path as well
as directly.  A new de-duplicating set is passed through within the
:meth:`.MetaData.reflect` sequence to prevent double reflection in this
way.

Change-Id: Ibf6650c1e76a44ccbe15765fd79df2fa53d6bac7
Fixes: #3861

3 files changed, 34 insertions, 11 deletions

diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py
index 719178f7e..ea3476295 100644
--- a/lib/sqlalchemy/engine/default.py
+++ b/lib/sqlalchemy/engine/default.py
@@ -366,9 +366,10 @@ class DefaultDialect(interfaces.Dialect):
         return sqltypes.adapt_type(typeobj, self.colspecs)
 
     def reflecttable(
-            self, connection, table, include_columns, exclude_columns):
+            self, connection, table, include_columns, exclude_columns, **opts):
         insp = reflection.Inspector.from_engine(connection)
-        return insp.reflecttable(table, include_columns, exclude_columns)
+        return insp.reflecttable(
+            table, include_columns, exclude_columns, **opts)
 
     def get_pk_constraint(self, conn, table_name, schema=None, **kw):
         """Compatibility method, adapts the result of get_primary_keys()
diff --git a/lib/sqlalchemy/engine/reflection.py b/lib/sqlalchemy/engine/reflection.py
index 5d11bf990..d1c9a22ac 100644
--- a/lib/sqlalchemy/engine/reflection.py
+++ b/lib/sqlalchemy/engine/reflection.py
@@ -532,7 +532,8 @@ class Inspector(object):
         return self.dialect.get_check_constraints(
             self.bind, table_name, schema, info_cache=self.info_cache, **kw)
 
-    def reflecttable(self, table, include_columns, exclude_columns=()):
+    def reflecttable(self, table, include_columns, exclude_columns=(),
+                     _extend_on=None):
         """Given a Table object, load its internal constructs based on
         introspection.
 
@@ -553,6 +554,13 @@ class Inspector(object):
           in the reflection process.  If ``None``, all columns are reflected.
 
         """
+
+        if _extend_on is not None:
+            if table in _extend_on:
+                return
+            else:
+                _extend_on.add(table)
+
         dialect = self.bind.dialect
 
         schema = self.bind.schema_for_object(table)
@@ -602,7 +610,7 @@ class Inspector(object):
 
         self._reflect_fk(
             table_name, schema, table, cols_by_orig_name,
-            exclude_columns, reflection_options)
+            exclude_columns, _extend_on, reflection_options)
 
         self._reflect_indexes(
             table_name, schema, table, cols_by_orig_name,
@@ -692,7 +700,7 @@ class Inspector(object):
 
     def _reflect_fk(
             self, table_name, schema, table, cols_by_orig_name,
-            exclude_columns, reflection_options):
+            exclude_columns, _extend_on, reflection_options):
         fkeys = self.get_foreign_keys(
             table_name, schema, **table.dialect_kwargs)
         for fkey_d in fkeys:
@@ -715,6 +723,7 @@ class Inspector(object):
                 sa_schema.Table(referred_table, table.metadata,
                                 autoload=True, schema=referred_schema,
                                 autoload_with=self.bind,
+                                _extend_on=_extend_on,
                                 **reflection_options
                                 )
                 for column in referred_columns:
@@ -724,6 +733,7 @@ class Inspector(object):
                 sa_schema.Table(referred_table, table.metadata, autoload=True,
                                 autoload_with=self.bind,
                                 schema=sa_schema.BLANK_SCHEMA,
+                                _extend_on=_extend_on,
                                 **reflection_options
                                 )
                 for column in referred_columns:
diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py
index 27ef9e01b..5c48e5bbb 100644
--- a/lib/sqlalchemy/sql/schema.py
+++ b/lib/sqlalchemy/sql/schema.py
@@ -485,6 +485,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)
+        _extend_on = kwargs.pop("_extend_on", None)
+
         include_columns = kwargs.pop('include_columns', None)
 
         self.implicit_returning = kwargs.pop('implicit_returning', True)
@@ -504,19 +506,22 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
         # we do it after the table is in the singleton dictionary to support
         # circular foreign keys
         if autoload:
-            self._autoload(metadata, autoload_with, include_columns)
+            self._autoload(
+                metadata, autoload_with,
+                include_columns, _extend_on=_extend_on)
 
         # initialize all the column, etc. objects.  done after reflection to
         # allow user-overrides
         self._init_items(*args)
 
     def _autoload(self, metadata, autoload_with, include_columns,
-                  exclude_columns=()):
+                  exclude_columns=(), _extend_on=None):
 
         if autoload_with:
             autoload_with.run_callable(
                 autoload_with.dialect.reflecttable,
-                self, include_columns, exclude_columns
+                self, include_columns, exclude_columns,
+                _extend_on=_extend_on
             )
         else:
             bind = _bind_or_error(
@@ -528,7 +533,8 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
                 "metadata.bind=<someengine>")
             bind.run_callable(
                 bind.dialect.reflecttable,
-                self, include_columns, exclude_columns
+                self, include_columns, exclude_columns,
+                _extend_on=_extend_on
             )
 
     @property
@@ -557,6 +563,8 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
         autoload = kwargs.pop('autoload', autoload_with is not None)
         autoload_replace = kwargs.pop('autoload_replace', True)
         schema = kwargs.pop('schema', None)
+        _extend_on = kwargs.pop('_extend_on', None)
+
         if schema and schema != self.schema:
             raise exc.ArgumentError(
                 "Can't change schema of existing table from '%s' to '%s'",
@@ -579,12 +587,15 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
 
         if autoload:
             if not autoload_replace:
+                # don't replace columns already present.
+                # we'd like to do this for constraints also however we don't
+                # have simple de-duping for unnamed constraints.
                 exclude_columns = [c.name for c in self.c]
             else:
                 exclude_columns = ()
             self._autoload(
                 self.metadata, autoload_with,
-                include_columns, exclude_columns)
+                include_columns, exclude_columns, _extend_on=_extend_on)
 
         self._extra_kwargs(**kwargs)
         self._init_items(*args)
@@ -3758,7 +3769,8 @@ class MetaData(SchemaItem):
                 'autoload': True,
                 'autoload_with': conn,
                 'extend_existing': extend_existing,
-                'autoload_replace': autoload_replace
+                'autoload_replace': autoload_replace,
+                '_extend_on': set()
             }
 
             reflect_opts.update(dialect_kwargs)