From: Mike Bayer Date: Wed, 18 May 2016 15:07:02 +0000 (-0400) Subject: Support "blank" schema when MetaData.schema is set X-Git-Tag: rel_1_1_0b1~40 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c124fa36d5af2c85c87c51d24e92387adffbe3d2;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 --- diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index 39670b0d8d..1b2414f2ad 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -15,6 +15,23 @@ .. include:: changelog_07.rst :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 5052e0e7fc..b37d579f1d 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 1193a1b0b0..b1d240edf9 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 eaa5e2e487..2d524978da 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 5b703f7b6a..bd0cbe54e6 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 5e709b1e3a..64692644cf 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 8da18108f9..4897c4a7e2 100644 --- a/test/dialect/postgresql/test_reflection.py +++ b/test/dialect/postgresql/test_reflection.py @@ -581,6 +581,29 @@ class ReflectionTest(fixtures.TestBase): eq_(set(meta3.tables), set( ['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 f9799fda0f..0d98147cb6 100644 --- a/test/engine/test_reflection.py +++ b/test/engine/test_reflection.py @@ -1303,6 +1303,31 @@ class SchemaTest(fixtures.TestBase): eq_(testing.db.dialect.has_schema(testing.db, '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') diff --git a/test/sql/test_metadata.py b/test/sql/test_metadata.py index 050929d3db..449956fcd4 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: