From: Mike Bayer Date: Sat, 29 Oct 2011 21:38:56 +0000 (-0400) Subject: - [bug] Postgresql dialect memoizes that an ENUM of a X-Git-Tag: rel_0_7_4~67 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e6a5ea8fa7d10078c8d3f1bf6cabc4399cac3e70;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - [bug] Postgresql dialect memoizes that an ENUM of a particular name was processed during a create/drop sequence. This allows a create/drop sequence to work without any calls to "checkfirst", and also means with "checkfirst" turned on it only needs to check for the ENUM once. [ticket:2311] --- diff --git a/CHANGES b/CHANGES index a2a3f2eb78..0ae1d192d1 100644 --- a/CHANGES +++ b/CHANGES @@ -81,6 +81,16 @@ CHANGES on dialect, but only works on Postgresql so far. Courtesy Manlio Perillo, [ticket:1679] +- postgresql + - [bug] Postgresql dialect memoizes that an ENUM of a + particular name was processed + during a create/drop sequence. This allows + a create/drop sequence to work without any + calls to "checkfirst", and also means with + "checkfirst" turned on it only needs to + check for the ENUM once. [ticket:2311] + + 0.7.3 ===== - general diff --git a/lib/sqlalchemy/dialects/postgresql/base.py b/lib/sqlalchemy/dialects/postgresql/base.py index 3ae60f6962..e3beeab79c 100644 --- a/lib/sqlalchemy/dialects/postgresql/base.py +++ b/lib/sqlalchemy/dialects/postgresql/base.py @@ -449,15 +449,40 @@ class ENUM(sqltypes.Enum): bind.dialect.has_type(bind, self.name, schema=self.schema): bind.execute(DropEnumType(self)) - def _on_table_create(self, event, target, bind, **kw): - self.create(bind=bind, checkfirst=True) + def _check_for_name_in_memos(self, checkfirst, kw): + """Look in the 'ddl runner' for 'memos', then + note our name in that collection. + + This to ensure a particular named enum is operated + upon only once within any kind of create/drop + sequence without relying upon "checkfirst". - def _on_metadata_create(self, event, target, bind, **kw): - if self.metadata is not None: - self.create(bind=bind, checkfirst=True) + """ + + if '_ddl_runner' in kw: + ddl_runner = kw['_ddl_runner'] + if '_pg_enums' in ddl_runner.memo: + pg_enums = ddl_runner.memo['_pg_enums'] + else: + pg_enums = ddl_runner.memo['_pg_enums'] = set() + present = self.name in pg_enums + pg_enums.add(self.name) + return present + else: + return False + + def _on_table_create(self, target, bind, checkfirst, **kw): + if not self._check_for_name_in_memos(checkfirst, kw): + self.create(bind=bind, checkfirst=checkfirst) + + def _on_metadata_create(self, target, bind, checkfirst, **kw): + if self.metadata is not None and \ + not self._check_for_name_in_memos(checkfirst, kw): + self.create(bind=bind, checkfirst=checkfirst) - def _on_metadata_drop(self, event, target, bind, **kw): - self.drop(bind=bind, checkfirst=True) + def _on_metadata_drop(self, target, bind, checkfirst, **kw): + if not self._check_for_name_in_memos(checkfirst, kw): + self.drop(bind=bind, checkfirst=checkfirst) colspecs = { sqltypes.Interval:INTERVAL, diff --git a/lib/sqlalchemy/engine/ddl.py b/lib/sqlalchemy/engine/ddl.py index 79958baae4..d6fdaee2ee 100644 --- a/lib/sqlalchemy/engine/ddl.py +++ b/lib/sqlalchemy/engine/ddl.py @@ -21,6 +21,7 @@ class SchemaGenerator(DDLBase): self.tables = tables and set(tables) or None self.preparer = dialect.identifier_preparer self.dialect = dialect + self.memo = {} def _can_create_table(self, table): self.dialect.validate_identifier(table.name) @@ -56,7 +57,8 @@ class SchemaGenerator(DDLBase): metadata.dispatch.before_create(metadata, self.connection, tables=collection, - checkfirst=self.checkfirst) + checkfirst=self.checkfirst, + _ddl_runner=self) for seq in seq_coll: self.traverse_single(seq, create_ok=True) @@ -66,14 +68,16 @@ class SchemaGenerator(DDLBase): metadata.dispatch.after_create(metadata, self.connection, tables=collection, - checkfirst=self.checkfirst) + checkfirst=self.checkfirst, + _ddl_runner=self) def visit_table(self, table, create_ok=False): if not create_ok and not self._can_create_table(table): return table.dispatch.before_create(table, self.connection, - checkfirst=self.checkfirst) + checkfirst=self.checkfirst, + _ddl_runner=self) for column in table.columns: if column.default is not None: @@ -86,7 +90,8 @@ class SchemaGenerator(DDLBase): self.traverse_single(index) table.dispatch.after_create(table, self.connection, - checkfirst=self.checkfirst) + checkfirst=self.checkfirst, + _ddl_runner=self) def visit_sequence(self, sequence, create_ok=False): if not create_ok and not self._can_create_sequence(sequence): @@ -104,6 +109,7 @@ class SchemaDropper(DDLBase): self.tables = tables self.preparer = dialect.identifier_preparer self.dialect = dialect + self.memo = {} def visit_metadata(self, metadata): if self.tables: @@ -117,7 +123,8 @@ class SchemaDropper(DDLBase): metadata.dispatch.before_drop(metadata, self.connection, tables=collection, - checkfirst=self.checkfirst) + checkfirst=self.checkfirst, + _ddl_runner=self) for table in collection: self.traverse_single(table, drop_ok=True) @@ -127,7 +134,8 @@ class SchemaDropper(DDLBase): metadata.dispatch.after_drop(metadata, self.connection, tables=collection, - checkfirst=self.checkfirst) + checkfirst=self.checkfirst, + _ddl_runner=self) def _can_drop_table(self, table): self.dialect.validate_identifier(table.name) @@ -155,7 +163,8 @@ class SchemaDropper(DDLBase): return table.dispatch.before_drop(table, self.connection, - checkfirst=self.checkfirst) + checkfirst=self.checkfirst, + _ddl_runner=self) for column in table.columns: if column.default is not None: @@ -164,7 +173,8 @@ class SchemaDropper(DDLBase): self.connection.execute(schema.DropTable(table)) table.dispatch.after_drop(table, self.connection, - checkfirst=self.checkfirst) + checkfirst=self.checkfirst, + _ddl_runner=self) def visit_sequence(self, sequence, drop_ok=False): if not drop_ok and not self._can_drop_sequence(sequence): diff --git a/lib/sqlalchemy/events.py b/lib/sqlalchemy/events.py index f7de851cc5..eee2a7557f 100644 --- a/lib/sqlalchemy/events.py +++ b/lib/sqlalchemy/events.py @@ -78,10 +78,11 @@ class DDLEvents(event.Events): :param connection: the :class:`.Connection` where the CREATE statement or statements will be emitted. :param \**kw: additional keyword arguments relevant - to the event. Currently this includes the ``tables`` - argument in the case of a :class:`.MetaData` object, - which is the list of :class:`.Table` objects for which - CREATE will be emitted. + to the event. The contents of this dictionary + may vary across releases, and include the + list of tables being generated for a metadata-level + event, the checkfirst flag, and other + elements used by internal events. """ @@ -93,10 +94,11 @@ class DDLEvents(event.Events): :param connection: the :class:`.Connection` where the CREATE statement or statements have been emitted. :param \**kw: additional keyword arguments relevant - to the event. Currently this includes the ``tables`` - argument in the case of a :class:`.MetaData` object, - which is the list of :class:`.Table` objects for which - CREATE has been emitted. + to the event. The contents of this dictionary + may vary across releases, and include the + list of tables being generated for a metadata-level + event, the checkfirst flag, and other + elements used by internal events. """ @@ -108,10 +110,11 @@ class DDLEvents(event.Events): :param connection: the :class:`.Connection` where the DROP statement or statements will be emitted. :param \**kw: additional keyword arguments relevant - to the event. Currently this includes the ``tables`` - argument in the case of a :class:`.MetaData` object, - which is the list of :class:`.Table` objects for which - DROP will be emitted. + to the event. The contents of this dictionary + may vary across releases, and include the + list of tables being generated for a metadata-level + event, the checkfirst flag, and other + elements used by internal events. """ @@ -123,10 +126,11 @@ class DDLEvents(event.Events): :param connection: the :class:`.Connection` where the DROP statement or statements have been emitted. :param \**kw: additional keyword arguments relevant - to the event. Currently this includes the ``tables`` - argument in the case of a :class:`.MetaData` object, - which is the list of :class:`.Table` objects for which - DROP has been emitted. + to the event. The contents of this dictionary + may vary across releases, and include the + list of tables being generated for a metadata-level + event, the checkfirst flag, and other + elements used by internal events. """ diff --git a/lib/sqlalchemy/types.py b/lib/sqlalchemy/types.py index af98dd8139..ae15a5d169 100644 --- a/lib/sqlalchemy/types.py +++ b/lib/sqlalchemy/types.py @@ -30,7 +30,7 @@ from sqlalchemy.util import pickle from sqlalchemy.util.compat import decimal from sqlalchemy.sql.visitors import Visitable from sqlalchemy import util -from sqlalchemy import processors, events +from sqlalchemy import processors, events, event import collections default = util.importlater("sqlalchemy.engine", "default") @@ -1686,26 +1686,45 @@ class SchemaType(events.SchemaEventTarget): self.schema = kw.pop('schema', None) self.metadata = kw.pop('metadata', None) if self.metadata: - self.metadata.append_ddl_listener('before-create', - util.portable_instancemethod(self._on_metadata_create)) - self.metadata.append_ddl_listener('after-drop', - util.portable_instancemethod(self._on_metadata_drop)) + event.listen( + self.metadata, + "before_create", + util.portable_instancemethod(self._on_metadata_create) + ) + event.listen( + self.metadata, + "after_drop", + util.portable_instancemethod(self._on_metadata_drop) + ) def _set_parent(self, column): column._on_table_attach(util.portable_instancemethod(self._set_table)) def _set_table(self, column, table): - table.append_ddl_listener('before-create', - util.portable_instancemethod( - self._on_table_create)) - table.append_ddl_listener('after-drop', - util.portable_instancemethod( - self._on_table_drop)) + event.listen( + table, + "before_create", + util.portable_instancemethod( + self._on_table_create) + ) + event.listen( + table, + "after_drop", + util.portable_instancemethod(self._on_table_drop) + ) if self.metadata is None: - table.metadata.append_ddl_listener('before-create', - util.portable_instancemethod(self._on_metadata_create)) - table.metadata.append_ddl_listener('after-drop', - util.portable_instancemethod(self._on_metadata_drop)) + # TODO: what's the difference between self.metadata + # and table.metadata here ? + event.listen( + table.metadata, + "before_create", + util.portable_instancemethod(self._on_metadata_create) + ) + event.listen( + table.metadata, + "after_drop", + util.portable_instancemethod(self._on_metadata_drop) + ) @property def bind(self): @@ -1729,25 +1748,25 @@ class SchemaType(events.SchemaEventTarget): if t.__class__ is not self.__class__ and isinstance(t, SchemaType): t.drop(bind=bind, checkfirst=checkfirst) - def _on_table_create(self, event, target, bind, **kw): + def _on_table_create(self, target, bind, **kw): t = self.dialect_impl(bind.dialect) if t.__class__ is not self.__class__ and isinstance(t, SchemaType): - t._on_table_create(event, target, bind, **kw) + t._on_table_create(target, bind, **kw) - def _on_table_drop(self, event, target, bind, **kw): + def _on_table_drop(self, target, bind, **kw): t = self.dialect_impl(bind.dialect) if t.__class__ is not self.__class__ and isinstance(t, SchemaType): - t._on_table_drop(event, target, bind, **kw) + t._on_table_drop(target, bind, **kw) - def _on_metadata_create(self, event, target, bind, **kw): + def _on_metadata_create(self, target, bind, **kw): t = self.dialect_impl(bind.dialect) if t.__class__ is not self.__class__ and isinstance(t, SchemaType): - t._on_metadata_create(event, target, bind, **kw) + t._on_metadata_create(target, bind, **kw) - def _on_metadata_drop(self, event, target, bind, **kw): + def _on_metadata_drop(self, target, bind, **kw): t = self.dialect_impl(bind.dialect) if t.__class__ is not self.__class__ and isinstance(t, SchemaType): - t._on_metadata_drop(event, target, bind, **kw) + t._on_metadata_drop(target, bind, **kw) class Enum(String, SchemaType): """Generic Enum Type. diff --git a/test/dialect/test_postgresql.py b/test/dialect/test_postgresql.py index 487139102d..b4b7e818cd 100644 --- a/test/dialect/test_postgresql.py +++ b/test/dialect/test_postgresql.py @@ -450,6 +450,30 @@ class EnumTest(fixtures.TestBase, AssertsExecutionResults, AssertsCompiledSQL): finally: metadata.drop_all(testing.db) + @testing.provide_metadata + def test_generate_multiple(self): + """Test that the same enum twice only generates once + for the create_all() call, without using checkfirst. + + A 'memo' collection held by the DDL runner + now handles this. + + """ + metadata = self.metadata + + e1 = Enum('one', 'two', 'three', + name="myenum") + t1 = Table('e1', metadata, + Column('c1', e1) + ) + + t2 = Table('e2', metadata, + Column('c1', e1) + ) + + metadata.create_all(checkfirst=False) + metadata.drop_all(checkfirst=False) + def test_non_native_dialect(self): engine = engines.testing_engine() engine.connect() diff --git a/test/lib/engines.py b/test/lib/engines.py index 649c3e31af..c8a44dc44e 100644 --- a/test/lib/engines.py +++ b/test/lib/engines.py @@ -247,12 +247,18 @@ def mock_engine(dialect_name=None): def assert_sql(stmts): recv = [re.sub(r'[\n\t]', '', str(s)) for s in buffer] assert recv == stmts, recv - + def print_sql(): + d = engine.dialect + return "\n".join( + str(s.compile(dialect=d)) + for s in engine.mock + ) engine = create_engine(dialect_name + '://', strategy='mock', executor=executor) assert not hasattr(engine, 'mock') engine.mock = buffer engine.assert_sql = assert_sql + engine.print_sql = print_sql return engine class DBAPIProxyCursor(object): diff --git a/test/sql/test_types.py b/test/sql/test_types.py index 50cb0ba066..987e97f9bc 100644 --- a/test/sql/test_types.py +++ b/test/sql/test_types.py @@ -956,6 +956,20 @@ class EnumTest(fixtures.TestBase): {'id':4, 'someenum':'four'} ) + def test_mock_engine_no_prob(self): + """ensure no 'checkfirst' queries are run when enums + are created with checkfirst=False""" + + e = engines.mock_engine() + t = Table('t1', MetaData(), + Column('x', Enum("x", "y", name="pge")) + ) + t.create(e, checkfirst=False) + # basically looking for the start of + # the constraint, or the ENUM def itself, + # depending on backend. + assert "('x'," in e.print_sql() + class BinaryTest(fixtures.TestBase, AssertsExecutionResults): __excluded_on__ = ( ('mysql', '<', (4, 1, 1)), # screwy varbinary types