Support "blank" schema when MetaData.schema is set

Previously, it was impossible to have a Table that has
None for a schema name when the "schema" parameter on
MetaData was set.  A new symbol sqlalchemy.schema.BLANK_SCHEMA
is added which indicates that the schema name should unconditionally
be set to None.  In particular, this value must be passed within
cross-schema foreign key reflection, so that a Table which
is in the "default" schema can be represented properly.

Fixes: #3716
Change-Id: I3d24f99c22cded206c5379fd32a225e74edb7a8e

9 files changed, 137 insertions, 5 deletions

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index 39670b0d8..1b2414f2a 100644
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -16,6 +16,23 @@
         :start-line: 5
 
 .. changelog::
+    :version: 1.0.14
+
+    .. change::
+        :tags: bug, engine, postgresql
+        :tickets: 3716
+
+        Fixed bug in cross-schema foreign key reflection in conjunction
+        with the :paramref:`.MetaData.schema` argument, where a referenced
+        table that is present in the "default" schema would fail since there
+        would be no way to indicate a :class:`.Table` that has "blank" for
+        a schema.  The special symbol :attr:`.schema.BLANK_SCHEMA` has been
+        added as an available value for :paramref:`.Table.schema` and
+        :paramref:`.Sequence.schema`, indicating that the schema name
+        should be forced to be ``None`` even if :paramref:`.MetaData.schema`
+        is specified.
+
+.. changelog::
     :version: 1.0.13
     :released: May 16, 2016
 
diff --git a/doc/build/core/metadata.rst b/doc/build/core/metadata.rst
index 5052e0e7f..b37d579f1 100644
--- a/doc/build/core/metadata.rst
+++ b/doc/build/core/metadata.rst
@@ -303,6 +303,23 @@ described in the individual documentation sections for each dialect.
 Column, Table, MetaData API
 ---------------------------
 
+.. attribute:: sqlalchemy.schema.BLANK_SCHEMA
+
+    Symbol indicating that a :class:`.Table` or :class:`.Sequence`
+    should have 'None' for its schema, even if the parent
+    :class:`.MetaData` has specified a schema.
+
+    .. seealso::
+
+        :paramref:`.MetaData.schema`
+
+        :paramref:`.Table.schema`
+
+        :paramref:`.Sequence.schema`
+
+    .. versionadded:: 1.0.14
+
+
 .. autoclass:: Column
     :members:
     :inherited-members:
diff --git a/lib/sqlalchemy/__init__.py b/lib/sqlalchemy/__init__.py
index 1193a1b0b..b1d240edf 100644
--- a/lib/sqlalchemy/__init__.py
+++ b/lib/sqlalchemy/__init__.py
@@ -120,6 +120,7 @@ from .schema import (
     ThreadLocalMetaData,
     UniqueConstraint,
     DDL,
+    BLANK_SCHEMA
 )
 
 
diff --git a/lib/sqlalchemy/engine/reflection.py b/lib/sqlalchemy/engine/reflection.py
index eaa5e2e48..2d524978d 100644
--- a/lib/sqlalchemy/engine/reflection.py
+++ b/lib/sqlalchemy/engine/reflection.py
@@ -693,6 +693,7 @@ class Inspector(object):
             else:
                 sa_schema.Table(referred_table, table.metadata, autoload=True,
                                 autoload_with=self.bind,
+                                schema=sa_schema.BLANK_SCHEMA,
                                 **reflection_options
                                 )
                 for column in referred_columns:
diff --git a/lib/sqlalchemy/schema.py b/lib/sqlalchemy/schema.py
index 5b703f7b6..bd0cbe54e 100644
--- a/lib/sqlalchemy/schema.py
+++ b/lib/sqlalchemy/schema.py
@@ -15,6 +15,7 @@ from .sql.base import (
 
 
 from .sql.schema import (
+    BLANK_SCHEMA,
     CheckConstraint,
     Column,
     ColumnDefault,
diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py
index 5e709b1e3..64692644c 100644
--- a/lib/sqlalchemy/sql/schema.py
+++ b/lib/sqlalchemy/sql/schema.py
@@ -46,6 +46,17 @@ from . import ddl
 
 RETAIN_SCHEMA = util.symbol('retain_schema')
 
+BLANK_SCHEMA = util.symbol(
+    'blank_schema',
+    """Symbol indicating that a :class:`.Table` or :class:`.Sequence`
+    should have 'None' for its schema, even if the parent
+    :class:`.MetaData` has specified a schema.
+
+    .. versionadded:: 1.0.14
+
+    """
+)
+
 
 def _get_table_key(name, schema):
     if schema is None:
@@ -340,6 +351,17 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
         the table resides in a schema other than the default selected schema
         for the engine's database connection.  Defaults to ``None``.
 
+        If the owning :class:`.MetaData` of this :class:`.Table` specifies
+        its own :paramref:`.MetaData.schema` parameter, then that schema
+        name will be applied to this :class:`.Table` if the schema parameter
+        here is set to ``None``.  To set a blank schema name on a :class:`.Table`
+        that would otherwise use the schema set on the owning :class:`.MetaData`,
+        specify the special symbol :attr:`.BLANK_SCHEMA`.
+
+        .. versionadded:: 1.0.14  Added the :attr:`.BLANK_SCHEMA` symbol to
+           allow a :class:`.Table` to have a blank schema name even when the
+           parent :class:`.MetaData` specifies :paramref:`.MetaData.schema`.
+
         The quoting rules for the schema name are the same as those for the
         ``name`` parameter, in that quoting is applied for reserved words or
         case-sensitive names; to enable unconditional quoting for the
@@ -371,6 +393,8 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
         schema = kw.get('schema', None)
         if schema is None:
             schema = metadata.schema
+        elif schema is BLANK_SCHEMA:
+            schema = None
         keep_existing = kw.pop('keep_existing', False)
         extend_existing = kw.pop('extend_existing', False)
         if 'useexisting' in kw:
@@ -442,6 +466,8 @@ class Table(DialectKWArgs, SchemaItem, TableClause):
         self.schema = kwargs.pop('schema', None)
         if self.schema is None:
             self.schema = metadata.schema
+        elif self.schema is BLANK_SCHEMA:
+            self.schema = None
         else:
             quote_schema = kwargs.pop('quote_schema', None)
             self.schema = quoted_name(self.schema, quote_schema)
@@ -2120,7 +2146,10 @@ class Sequence(DefaultGenerator):
          .. versionadded:: 1.0.7
 
         :param schema: Optional schema name for the sequence, if located
-         in a schema other than the default.
+         in a schema other than the default.  The rules for selecting the
+         schema name when a :class:`.MetaData` is also present are the same
+         as that of :paramref:`.Table.schema`.
+
         :param optional: boolean value, when ``True``, indicates that this
          :class:`.Sequence` object only needs to be explicitly generated
          on backends that don't provide another way to generate primary
@@ -2169,7 +2198,9 @@ class Sequence(DefaultGenerator):
         self.nomaxvalue = nomaxvalue
         self.cycle = cycle
         self.optional = optional
-        if metadata is not None and schema is None and metadata.schema:
+        if schema is BLANK_SCHEMA:
+            self.schema = schema = None
+        elif metadata is not None and schema is None and metadata.schema:
             self.schema = schema = metadata.schema
         else:
             self.schema = quoted_name(schema, quote_schema)
@@ -3372,8 +3403,21 @@ class MetaData(SchemaItem):
 
         :param schema:
            The default schema to use for the :class:`.Table`,
-           :class:`.Sequence`, and other objects associated with this
-           :class:`.MetaData`. Defaults to ``None``.
+           :class:`.Sequence`, and potentially other objects associated with
+           this :class:`.MetaData`. Defaults to ``None``.
+
+           When this value is set, any :class:`.Table` or :class:`.Sequence`
+           which specifies ``None`` for the schema parameter will instead
+           have this schema name defined.  To build a :class:`.Table`
+           or :class:`.Sequence` that still has ``None`` for the schema
+           even when this parameter is present, use the :attr:`.BLANK_SCHEMA`
+           symbol.
+
+           .. seealso::
+
+                :paramref:`.Table.schema`
+
+                :paramref:`.Sequence.schema`
 
         :param quote_schema:
             Sets the ``quote_schema`` flag for those :class:`.Table`,
diff --git a/test/dialect/postgresql/test_reflection.py b/test/dialect/postgresql/test_reflection.py
index 8da18108f..4897c4a7e 100644
--- a/test/dialect/postgresql/test_reflection.py
+++ b/test/dialect/postgresql/test_reflection.py
@@ -582,6 +582,29 @@ class ReflectionTest(fixtures.TestBase):
                 ['test_schema_2.some_other_table', 'test_schema.some_table']))
 
     @testing.provide_metadata
+    def test_cross_schema_reflection_metadata_uses_schema(self):
+        # test [ticket:3716]
+
+        metadata = self.metadata
+
+        Table('some_table', metadata,
+              Column('id', Integer, primary_key=True),
+              Column('sid', Integer, ForeignKey('some_other_table.id')),
+              schema='test_schema'
+              )
+        Table('some_other_table', metadata,
+              Column('id', Integer, primary_key=True),
+              schema=None
+              )
+        metadata.create_all()
+        with testing.db.connect() as conn:
+            meta2 = MetaData(conn, schema="test_schema")
+            meta2.reflect()
+
+            eq_(set(meta2.tables), set(
+                ['some_other_table', 'test_schema.some_table']))
+
+    @testing.provide_metadata
     def test_uppercase_lowercase_table(self):
         metadata = self.metadata
 
diff --git a/test/engine/test_reflection.py b/test/engine/test_reflection.py
index f9799fda0..0d98147cb 100644
--- a/test/engine/test_reflection.py
+++ b/test/engine/test_reflection.py
@@ -1304,6 +1304,31 @@ class SchemaTest(fixtures.TestBase):
             'sa_fake_schema_123'), False)
 
     @testing.requires.schemas
+    @testing.requires.cross_schema_fk_reflection
+    @testing.provide_metadata
+    def test_blank_schema_arg(self):
+        metadata = self.metadata
+
+        Table('some_table', metadata,
+              Column('id', Integer, primary_key=True),
+              Column('sid', Integer, sa.ForeignKey('some_other_table.id')),
+              schema=testing.config.test_schema
+              )
+        Table('some_other_table', metadata,
+              Column('id', Integer, primary_key=True),
+              schema=None
+              )
+        metadata.create_all()
+        with testing.db.connect() as conn:
+            meta2 = MetaData(conn, schema=testing.config.test_schema)
+            meta2.reflect()
+
+            eq_(set(meta2.tables), set(
+                [
+                    'some_other_table',
+                    '%s.some_table' % testing.config.test_schema]))
+
+    @testing.requires.schemas
     @testing.fails_on('sqlite', 'FIXME: unknown')
     @testing.fails_on('sybase', 'FIXME: unknown')
     def test_explicit_default_schema(self):
diff --git a/test/sql/test_metadata.py b/test/sql/test_metadata.py
index 050929d3d..449956fcd 100644
--- a/test/sql/test_metadata.py
+++ b/test/sql/test_metadata.py
@@ -7,7 +7,8 @@ from sqlalchemy import Integer, String, UniqueConstraint, \
     CheckConstraint, ForeignKey, MetaData, Sequence, \
     ForeignKeyConstraint, PrimaryKeyConstraint, ColumnDefault, Index, event,\
     events, Unicode, types as sqltypes, bindparam, \
-    Table, Column, Boolean, Enum, func, text, TypeDecorator
+    Table, Column, Boolean, Enum, func, text, TypeDecorator, \
+    BLANK_SCHEMA
 from sqlalchemy import schema, exc
 from sqlalchemy.engine import default
 from sqlalchemy.sql import elements, naming
@@ -446,6 +447,7 @@ class MetaDataTest(fixtures.TestBase, ComparesTables):
                     ('t2', m1, 'sch2', None, 'sch2', None),
                     ('t3', m1, 'sch2', True, 'sch2', True),
                     ('t4', m1, 'sch1', None, 'sch1', None),
+                    ('t5', m1, BLANK_SCHEMA, None, None, None),
                     ('t1', m2, None, None, 'sch1', True),
                     ('t2', m2, 'sch2', None, 'sch2', None),
                     ('t3', m2, 'sch2', True, 'sch2', True),
@@ -458,6 +460,7 @@ class MetaDataTest(fixtures.TestBase, ComparesTables):
                     ('t2', m4, 'sch2', None, 'sch2', None),
                     ('t3', m4, 'sch2', True, 'sch2', True),
                     ('t4', m4, 'sch1', None, 'sch1', None),
+                    ('t5', m4, BLANK_SCHEMA, None, None, None),
                 ]):
             kw = {}
             if schema is not None: