From: Mike Bayer Date: Tue, 6 Aug 2019 20:10:09 +0000 (-0400) Subject: Implement checkfirst for Index.create(), Index.drop() X-Git-Tag: rel_1_4_0b1~771 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d8da7f5ac544f3dd853a221faa5fab4ff788e25b;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Implement checkfirst for Index.create(), Index.drop() The :meth:`.Index.create` and :meth:`.Index.drop` methods now have a parameter :paramref:`.Index.create.checkfirst`, in the same way as that of :class:`.Table` and :class:`.Sequence`, which when enabled will cause the operation to detect if the index exists (or not) before performing a create or drop operation. Fixes: #527 Change-Id: Idf994bc016359d0ae86cc64ccb20378115cb66d6 --- diff --git a/doc/build/changelog/unreleased_14/527.rst b/doc/build/changelog/unreleased_14/527.rst new file mode 100644 index 0000000000..83f59daf5b --- /dev/null +++ b/doc/build/changelog/unreleased_14/527.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: usecase, sql + :tickets: 527 + + The :meth:`.Index.create` and :meth:`.Index.drop` methods now have a + parameter :paramref:`.Index.create.checkfirst`, in the same way as that of + :class:`.Table` and :class:`.Sequence`, which when enabled will cause the + operation to detect if the index exists (or not) before performing a create + or drop operation. + diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py index 9d457b8009..fb1728eab8 100644 --- a/lib/sqlalchemy/engine/default.py +++ b/lib/sqlalchemy/engine/default.py @@ -426,6 +426,15 @@ class DefaultDialect(interfaces.Dialect): ) } + def has_index(self, connection, table_name, index_name, schema=None): + if not self.has_table(connection, table_name, schema=schema): + return False + for idx in self.get_indexes(connection, table_name, schema=schema): + if idx["name"] == index_name: + return True + else: + return False + def validate_identifier(self, ident): if len(ident) > self.max_identifier_length: raise exc.IdentifierError( diff --git a/lib/sqlalchemy/engine/interfaces.py b/lib/sqlalchemy/engine/interfaces.py index 5bd3b1d3e6..fddc1712f3 100644 --- a/lib/sqlalchemy/engine/interfaces.py +++ b/lib/sqlalchemy/engine/interfaces.py @@ -439,6 +439,24 @@ class Dialect(object): raise NotImplementedError() + def has_index(self, connection, table_name, index_name, schema=None): + """Check the existence of a particular index name in the database. + + Given a :class:`.Connection` object, a string + `table_name` and stiring index name, return True if an index of the + given name on the given table exists, false otherwise. + + The :class:`.DefaultDialect` implements this in terms of the + :meth:`.Dialect.has_table` and :meth:`.Dialect.get_indexes` methods, + however dialects can implement a more performant version. + + + .. versionadded:: 1.4 + + """ + + raise NotImplementedError() + def has_sequence(self, connection, sequence_name, schema=None, **kw): """Check the existence of a particular sequence in the database. diff --git a/lib/sqlalchemy/sql/ddl.py b/lib/sqlalchemy/sql/ddl.py index ff36a68e4a..ef6614b615 100644 --- a/lib/sqlalchemy/sql/ddl.py +++ b/lib/sqlalchemy/sql/ddl.py @@ -732,6 +732,17 @@ class SchemaGenerator(DDLBase): self.connection, table.name, schema=effective_schema ) + def _can_create_index(self, index): + effective_schema = self.connection.schema_for_object(index.table) + if effective_schema: + self.dialect.validate_identifier(effective_schema) + return not self.checkfirst or not self.dialect.has_index( + self.connection, + index.table.name, + index.name, + schema=effective_schema, + ) + def _can_create_sequence(self, sequence): effective_schema = self.connection.schema_for_object(sequence) @@ -831,7 +842,7 @@ class SchemaGenerator(DDLBase): if hasattr(table, "indexes"): for index in table.indexes: - self.traverse_single(index) + self.traverse_single(index, create_ok=True) if self.dialect.supports_comments and not self.dialect.inline_comments: if table.comment is not None: @@ -859,7 +870,9 @@ class SchemaGenerator(DDLBase): return self.connection.execute(CreateSequence(sequence)) - def visit_index(self, index): + def visit_index(self, index, create_ok=False): + if not create_ok and not self._can_create_index(index): + return self.connection.execute(CreateIndex(index)) @@ -973,6 +986,17 @@ class SchemaDropper(DDLBase): self.connection, table.name, schema=effective_schema ) + def _can_drop_index(self, index): + effective_schema = self.connection.schema_for_object(index.table) + if effective_schema: + self.dialect.validate_identifier(effective_schema) + return not self.checkfirst or self.dialect.has_index( + self.connection, + index.table.name, + index.name, + schema=effective_schema, + ) + def _can_drop_sequence(self, sequence): effective_schema = self.connection.schema_for_object(sequence) return self.dialect.supports_sequences and ( @@ -985,7 +1009,10 @@ class SchemaDropper(DDLBase): ) ) - def visit_index(self, index): + def visit_index(self, index, drop_ok=False): + if not drop_ok and not self._can_drop_index(index): + return + self.connection.execute(DropIndex(index)) def visit_table(self, table, drop_ok=False, _is_metadata_operation=False): diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py index 0910128960..d9667e4455 100644 --- a/lib/sqlalchemy/sql/schema.py +++ b/lib/sqlalchemy/sql/schema.py @@ -3670,7 +3670,7 @@ class Index(DialectKWArgs, ColumnCollectionMixin, SchemaItem): return self.table.bind - def create(self, bind=None): + def create(self, bind=None, checkfirst=False): """Issue a ``CREATE`` statement for this :class:`.Index`, using the given :class:`.Connectable` for connectivity. @@ -3682,10 +3682,10 @@ class Index(DialectKWArgs, ColumnCollectionMixin, SchemaItem): """ if bind is None: bind = _bind_or_error(self) - bind._run_ddl_visitor(ddl.SchemaGenerator, self) + bind._run_ddl_visitor(ddl.SchemaGenerator, self, checkfirst=checkfirst) return self - def drop(self, bind=None): + def drop(self, bind=None, checkfirst=False): """Issue a ``DROP`` statement for this :class:`.Index`, using the given :class:`.Connectable` for connectivity. @@ -3697,7 +3697,7 @@ class Index(DialectKWArgs, ColumnCollectionMixin, SchemaItem): """ if bind is None: bind = _bind_or_error(self) - bind._run_ddl_visitor(ddl.SchemaDropper, self) + bind._run_ddl_visitor(ddl.SchemaDropper, self, checkfirst=checkfirst) def __repr__(self): return "Index(%s)" % ( diff --git a/lib/sqlalchemy/testing/suite/test_reflection.py b/lib/sqlalchemy/testing/suite/test_reflection.py index 9ca13ec4e5..9b6cec10c6 100644 --- a/lib/sqlalchemy/testing/suite/test_reflection.py +++ b/lib/sqlalchemy/testing/suite/test_reflection.py @@ -77,6 +77,65 @@ class HasTableTest(fixtures.TablesTest): ) +class HasIndexTest(fixtures.TablesTest): + __backend__ = True + + @classmethod + def define_tables(cls, metadata): + tt = Table( + "test_table", + metadata, + Column("id", Integer, primary_key=True), + Column("data", String(50)), + ) + Index("my_idx", tt.c.data) + + if testing.requires.schemas.enabled: + tt = Table( + "test_table", + metadata, + Column("id", Integer, primary_key=True), + Column("data", String(50)), + schema=config.test_schema, + ) + Index("my_idx_s", tt.c.data) + + def test_has_index(self): + with config.db.begin() as conn: + assert config.db.dialect.has_index(conn, "test_table", "my_idx") + assert not config.db.dialect.has_index( + conn, "test_table", "my_idx_s" + ) + assert not config.db.dialect.has_index( + conn, "nonexistent_table", "my_idx" + ) + assert not config.db.dialect.has_index( + conn, "test_table", "nonexistent_idx" + ) + + @testing.requires.schemas + def test_has_index_schema(self): + with config.db.begin() as conn: + assert config.db.dialect.has_index( + conn, "test_table", "my_idx_s", schema=config.test_schema + ) + assert not config.db.dialect.has_index( + conn, "test_table", "my_idx", schema=config.test_schema + ) + assert not config.db.dialect.has_index( + conn, + "nonexistent_table", + "my_idx_s", + schema=config.test_schema, + ) + assert not config.db.dialect.has_index( + conn, + "test_table", + "nonexistent_idx_s", + schema=config.test_schema, + ) + + class ComponentReflectionTest(fixtures.TablesTest): run_inserts = run_deletes = None @@ -1129,4 +1188,9 @@ class NormalizedNameTest(fixtures.TablesTest): eq_(tablenames[1].upper(), tablenames[1].lower()) -__all__ = ("ComponentReflectionTest", "HasTableTest", "NormalizedNameTest") +__all__ = ( + "ComponentReflectionTest", + "HasTableTest", + "HasIndexTest", + "NormalizedNameTest", +) diff --git a/test/sql/test_ddlemit.py b/test/sql/test_ddlemit.py index 3cdbfaeccb..13300f0b58 100644 --- a/test/sql/test_ddlemit.py +++ b/test/sql/test_ddlemit.py @@ -1,5 +1,6 @@ from sqlalchemy import Column from sqlalchemy import ForeignKey +from sqlalchemy import Index from sqlalchemy import Integer from sqlalchemy import MetaData from sqlalchemy import schema @@ -16,11 +17,15 @@ class EmitDDLTest(fixtures.TestBase): def has_item(connection, name, schema): return item_exists(name) + def has_index(connection, tablename, idxname, schema): + return item_exists(idxname) + return Mock( dialect=Mock( supports_sequences=True, has_table=Mock(side_effect=has_item), has_sequence=Mock(side_effect=has_item), + has_index=Mock(side_effect=has_index), supports_comments=True, inline_comments=False, ) @@ -86,6 +91,12 @@ class EmitDDLTest(fixtures.TestBase): t2 = Table("t2", m, Column("id", Integer, primary_key=True)) return m, t1, t2 + def _table_index_fixture(self): + m = MetaData() + t1 = Table("t1", m, Column("x", Integer), Column("y", Integer)) + i1 = Index("my_idx", t1.c.x, t1.c.y) + return m, t1, i1 + def _table_seq_fixture(self): m = MetaData() @@ -130,6 +141,105 @@ class EmitDDLTest(fixtures.TestBase): self._assert_drop([t1, s1], generator, m) + def test_create_table_index_checkfirst(self): + """create table that doesn't exist should not require a check + on the index""" + + m, t1, i1 = self._table_index_fixture() + + def exists(name): + if name == "my_idx": + raise NotImplementedError() + else: + return False + + generator = self._mock_create_fixture(True, [t1], item_exists=exists) + self._assert_create([t1, i1], generator, t1) + + def test_create_table_exists_index_checkfirst(self): + """for the moment, if the table *does* exist, we are not checking + for the index. this can possibly be changed.""" + + m, t1, i1 = self._table_index_fixture() + + def exists(name): + if name == "my_idx": + raise NotImplementedError() + else: + return True + + generator = self._mock_create_fixture(True, [t1], item_exists=exists) + # nothing is created + self._assert_create([], generator, t1) + + def test_drop_table_index_checkfirst(self): + m, t1, i1 = self._table_index_fixture() + + def exists(name): + if name == "my_idx": + raise NotImplementedError() + else: + return True + + generator = self._mock_drop_fixture(True, [t1], item_exists=exists) + self._assert_drop_tables([t1], generator, t1) + + def test_create_index_checkfirst_exists(self): + m, t1, i1 = self._table_index_fixture() + generator = self._mock_create_fixture( + True, [i1], item_exists=lambda idx: True + ) + self._assert_create_index([], generator, i1) + + def test_create_index_checkfirst_doesnt_exist(self): + m, t1, i1 = self._table_index_fixture() + generator = self._mock_create_fixture( + True, [i1], item_exists=lambda idx: False + ) + self._assert_create_index([i1], generator, i1) + + def test_create_index_nocheck_exists(self): + m, t1, i1 = self._table_index_fixture() + generator = self._mock_create_fixture( + False, [i1], item_exists=lambda idx: True + ) + self._assert_create_index([i1], generator, i1) + + def test_create_index_nocheck_doesnt_exist(self): + m, t1, i1 = self._table_index_fixture() + generator = self._mock_create_fixture( + False, [i1], item_exists=lambda idx: False + ) + self._assert_create_index([i1], generator, i1) + + def test_drop_index_checkfirst_exists(self): + m, t1, i1 = self._table_index_fixture() + generator = self._mock_drop_fixture( + True, [i1], item_exists=lambda idx: True + ) + self._assert_drop_index([i1], generator, i1) + + def test_drop_index_checkfirst_doesnt_exist(self): + m, t1, i1 = self._table_index_fixture() + generator = self._mock_drop_fixture( + True, [i1], item_exists=lambda idx: False + ) + self._assert_drop_index([], generator, i1) + + def test_drop_index_nocheck_exists(self): + m, t1, i1 = self._table_index_fixture() + generator = self._mock_drop_fixture( + False, [i1], item_exists=lambda idx: True + ) + self._assert_drop_index([i1], generator, i1) + + def test_drop_index_nocheck_doesnt_exist(self): + m, t1, i1 = self._table_index_fixture() + generator = self._mock_drop_fixture( + False, [i1], item_exists=lambda idx: False + ) + self._assert_drop_index([i1], generator, i1) + def test_create_collection_checkfirst(self): m, t1, t2, t3, t4, t5 = self._table_fixture() generator = self._mock_create_fixture( @@ -240,7 +350,7 @@ class EmitDDLTest(fixtures.TestBase): def _assert_create(self, elements, generator, argument): self._assert_ddl( - (schema.CreateTable, schema.CreateSequence), + (schema.CreateTable, schema.CreateSequence, schema.CreateIndex), elements, generator, argument, @@ -282,6 +392,12 @@ class EmitDDLTest(fixtures.TestBase): argument, ) + def _assert_create_index(self, elements, generator, argument): + self._assert_ddl((schema.CreateIndex,), elements, generator, argument) + + def _assert_drop_index(self, elements, generator, argument): + self._assert_ddl((schema.DropIndex,), elements, generator, argument) + def _assert_ddl(self, ddl_cls, elements, generator, argument): generator.traverse_single(argument) for call_ in generator.connection.execute.mock_calls: