From dc26fd51495348b3e9f7a8a2e39c3fa8b2091d76 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 8 Feb 2021 12:22:38 -1000 Subject: [PATCH] Ensure creating an index that already exists is forgiving for postgresql (#46185) Unlikely sqlite and mysql, postgresql throws ProgrammingError instead of InternalError or OperationalError when trying to create an index that already exists. --- .../components/recorder/migration.py | 16 +++++----- tests/components/recorder/test_migrate.py | 30 ++++++++++++++++++- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/homeassistant/components/recorder/migration.py b/homeassistant/components/recorder/migration.py index 4501b25385e..aeb62cc111d 100644 --- a/homeassistant/components/recorder/migration.py +++ b/homeassistant/components/recorder/migration.py @@ -3,7 +3,12 @@ import logging from sqlalchemy import ForeignKeyConstraint, MetaData, Table, text from sqlalchemy.engine import reflection -from sqlalchemy.exc import InternalError, OperationalError, SQLAlchemyError +from sqlalchemy.exc import ( + InternalError, + OperationalError, + ProgrammingError, + SQLAlchemyError, +) from sqlalchemy.schema import AddConstraint, DropConstraint from .const import DOMAIN @@ -69,7 +74,7 @@ def _create_index(engine, table_name, index_name): ) try: index.create(engine) - except OperationalError as err: + except (InternalError, ProgrammingError, OperationalError) as err: lower_err_str = str(err).lower() if "already exists" not in lower_err_str and "duplicate" not in lower_err_str: @@ -78,13 +83,6 @@ def _create_index(engine, table_name, index_name): _LOGGER.warning( "Index %s already exists on %s, continuing", index_name, table_name ) - except InternalError as err: - if "duplicate" not in str(err).lower(): - raise - - _LOGGER.warning( - "Index %s already exists on %s, continuing", index_name, table_name - ) _LOGGER.debug("Finished creating %s", index_name) diff --git a/tests/components/recorder/test_migrate.py b/tests/components/recorder/test_migrate.py index d10dad43d75..c29dad2d495 100644 --- a/tests/components/recorder/test_migrate.py +++ b/tests/components/recorder/test_migrate.py @@ -1,9 +1,10 @@ """The tests for the Recorder component.""" # pylint: disable=protected-access -from unittest.mock import call, patch +from unittest.mock import Mock, PropertyMock, call, patch import pytest from sqlalchemy import create_engine +from sqlalchemy.exc import InternalError, OperationalError, ProgrammingError from sqlalchemy.pool import StaticPool from homeassistant.bootstrap import async_setup_component @@ -79,3 +80,30 @@ def test_forgiving_add_index(): engine = create_engine("sqlite://", poolclass=StaticPool) models.Base.metadata.create_all(engine) migration._create_index(engine, "states", "ix_states_context_id") + + +@pytest.mark.parametrize( + "exception_type", [OperationalError, ProgrammingError, InternalError] +) +def test_forgiving_add_index_with_other_db_types(caplog, exception_type): + """Test that add index will continue if index exists on mysql and postgres.""" + mocked_index = Mock() + type(mocked_index).name = "ix_states_context_id" + mocked_index.create = Mock( + side_effect=exception_type( + "CREATE INDEX ix_states_old_state_id ON states (old_state_id);", + [], + 'relation "ix_states_old_state_id" already exists', + ) + ) + + mocked_table = Mock() + type(mocked_table).indexes = PropertyMock(return_value=[mocked_index]) + + with patch( + "homeassistant.components.recorder.migration.Table", return_value=mocked_table + ): + migration._create_index(Mock(), "states", "ix_states_context_id") + + assert "already exists on states" in caplog.text + assert "continuing" in caplog.text