mirror of
https://github.com/home-assistant/core.git
synced 2025-07-15 17:27:10 +00:00
Fix minimum schema version to run event_id_post_migration (#139014)
* Fix minimum version to run event_id_post_migration The table rebuild to fix the foreign key constraint was added in https://github.com/home-assistant/core/pull/120779 but the schema version was not bumped so we need to make sure any database that was created with schema 43 or older still has the migration run as otherwise they will not be able to purge the database with SQLite since each delete in the events table will due a full table scan of the states table to look for a foreign key that is not there fixes #138818 * Apply suggestions from code review * Update homeassistant/components/recorder/migration.py * Update homeassistant/components/recorder/migration.py * Update homeassistant/components/recorder/const.py * Apply suggestions from code review * Apply suggestions from code review * Apply suggestions from code review * Apply suggestions from code review * update tests, add more cover * update tests, add more cover * Update tests/components/recorder/test_migration_run_time_migrations_remember.py
This commit is contained in:
J. Nick Koston
GitHub
parent
98c6a578b7
commit
93b01a3bc3
@ -50,6 +50,11 @@ STATES_META_SCHEMA_VERSION = 38
|
||||
LAST_REPORTED_SCHEMA_VERSION = 43
|
||||
|
||||
LEGACY_STATES_EVENT_ID_INDEX_SCHEMA_VERSION = 28
|
||||
LEGACY_STATES_EVENT_FOREIGN_KEYS_FIXED_SCHEMA_VERSION = 43
|
||||
# https://github.com/home-assistant/core/pull/120779
|
||||
# fixed the foreign keys in the states table but it did
|
||||
# not bump the schema version which means only databases
|
||||
# created with schema 44 and later do not need the rebuild.
|
||||
|
||||
INTEGRATION_PLATFORM_COMPILE_STATISTICS = "compile_statistics"
|
||||
INTEGRATION_PLATFORM_LIST_STATISTIC_IDS = "list_statistic_ids"
|
||||
|
||||
@ -52,6 +52,7 @@ from .auto_repairs.statistics.schema import (
|
||||
from .const import (
|
||||
CONTEXT_ID_AS_BINARY_SCHEMA_VERSION,
|
||||
EVENT_TYPE_IDS_SCHEMA_VERSION,
|
||||
LEGACY_STATES_EVENT_FOREIGN_KEYS_FIXED_SCHEMA_VERSION,
|
||||
LEGACY_STATES_EVENT_ID_INDEX_SCHEMA_VERSION,
|
||||
STATES_META_SCHEMA_VERSION,
|
||||
SupportedDialect,
|
||||
@ -2490,9 +2491,10 @@ class BaseMigration(ABC):
|
||||
if self.initial_schema_version > self.max_initial_schema_version:
|
||||
_LOGGER.debug(
|
||||
"Data migration '%s' not needed, database created with version %s "
|
||||
"after migrator was added",
|
||||
"after migrator was added in version %s",
|
||||
self.migration_id,
|
||||
self.initial_schema_version,
|
||||
self.max_initial_schema_version,
|
||||
)
|
||||
return False
|
||||
if self.start_schema_version < self.required_schema_version:
|
||||
@ -2868,7 +2870,14 @@ class EventIDPostMigration(BaseRunTimeMigration):
|
||||
"""Migration to remove old event_id index from states."""
|
||||
|
||||
migration_id = "event_id_post_migration"
|
||||
max_initial_schema_version = LEGACY_STATES_EVENT_ID_INDEX_SCHEMA_VERSION - 1
|
||||
# Note we don't subtract 1 from the max_initial_schema_version
|
||||
# in this case because we need to run this migration on databases
|
||||
# version >= 43 because the schema was not bumped when the table
|
||||
# rebuild was added in
|
||||
# https://github.com/home-assistant/core/pull/120779
|
||||
# which means its only safe to assume version 44 and later
|
||||
# do not need the table rebuild
|
||||
max_initial_schema_version = LEGACY_STATES_EVENT_FOREIGN_KEYS_FIXED_SCHEMA_VERSION
|
||||
task = MigrationTask
|
||||
migration_version = 2
|
||||
|
||||
|
||||
@ -225,6 +225,7 @@ async def test_migrate_events_context_ids(
|
||||
patch.object(recorder, "db_schema", old_db_schema),
|
||||
patch.object(migration, "SCHEMA_VERSION", old_db_schema.SCHEMA_VERSION),
|
||||
patch.object(migration.EventsContextIDMigration, "migrate_data"),
|
||||
patch.object(migration.EventIDPostMigration, "migrate_data"),
|
||||
patch(CREATE_ENGINE_TARGET, new=_create_engine_test),
|
||||
):
|
||||
async with (
|
||||
@ -282,6 +283,7 @@ async def test_migrate_events_context_ids(
|
||||
patch(
|
||||
"sqlalchemy.schema.Index.create", autospec=True, wraps=Index.create
|
||||
) as wrapped_idx_create,
|
||||
patch.object(migration.EventIDPostMigration, "migrate_data"),
|
||||
):
|
||||
async with async_test_recorder(
|
||||
hass, wait_recorder=False, wait_recorder_setup=False
|
||||
@ -588,6 +590,7 @@ async def test_migrate_states_context_ids(
|
||||
patch.object(recorder, "db_schema", old_db_schema),
|
||||
patch.object(migration, "SCHEMA_VERSION", old_db_schema.SCHEMA_VERSION),
|
||||
patch.object(migration.StatesContextIDMigration, "migrate_data"),
|
||||
patch.object(migration.EventIDPostMigration, "migrate_data"),
|
||||
patch(CREATE_ENGINE_TARGET, new=_create_engine_test),
|
||||
):
|
||||
async with (
|
||||
@ -640,6 +643,7 @@ async def test_migrate_states_context_ids(
|
||||
patch(
|
||||
"sqlalchemy.schema.Index.create", autospec=True, wraps=Index.create
|
||||
) as wrapped_idx_create,
|
||||
patch.object(migration.EventIDPostMigration, "migrate_data"),
|
||||
):
|
||||
async with async_test_recorder(
|
||||
hass, wait_recorder=False, wait_recorder_setup=False
|
||||
@ -1127,6 +1131,7 @@ async def test_post_migrate_entity_ids(
|
||||
patch.object(migration, "SCHEMA_VERSION", old_db_schema.SCHEMA_VERSION),
|
||||
patch.object(migration.EntityIDMigration, "migrate_data"),
|
||||
patch.object(migration.EntityIDPostMigration, "migrate_data"),
|
||||
patch.object(migration.EventIDPostMigration, "migrate_data"),
|
||||
patch(CREATE_ENGINE_TARGET, new=_create_engine_test),
|
||||
):
|
||||
async with (
|
||||
@ -1158,9 +1163,12 @@ async def test_post_migrate_entity_ids(
|
||||
return {state.state: state.entity_id for state in states}
|
||||
|
||||
# Run again with new schema, let migration run
|
||||
with patch(
|
||||
with (
|
||||
patch(
|
||||
"sqlalchemy.schema.Index.create", autospec=True, wraps=Index.create
|
||||
) as wrapped_idx_create:
|
||||
) as wrapped_idx_create,
|
||||
patch.object(migration.EventIDPostMigration, "migrate_data"),
|
||||
):
|
||||
async with (
|
||||
async_test_home_assistant() as hass,
|
||||
async_test_recorder(hass) as instance,
|
||||
@ -1169,7 +1177,6 @@ async def test_post_migrate_entity_ids(
|
||||
|
||||
await hass.async_block_till_done()
|
||||
await async_wait_recording_done(hass)
|
||||
await async_wait_recording_done(hass)
|
||||
|
||||
states_by_state = await instance.async_add_executor_job(
|
||||
_fetch_migrated_states
|
||||
|
||||
@ -115,7 +115,7 @@ def _create_engine_test(
|
||||
"event_context_id_as_binary": (0, 1),
|
||||
"event_type_id_migration": (2, 1),
|
||||
"entity_id_migration": (2, 1),
|
||||
"event_id_post_migration": (0, 0),
|
||||
"event_id_post_migration": (1, 1),
|
||||
"entity_id_post_migration": (0, 1),
|
||||
},
|
||||
[
|
||||
@ -131,7 +131,7 @@ def _create_engine_test(
|
||||
"event_context_id_as_binary": (0, 0),
|
||||
"event_type_id_migration": (2, 1),
|
||||
"entity_id_migration": (2, 1),
|
||||
"event_id_post_migration": (0, 0),
|
||||
"event_id_post_migration": (1, 1),
|
||||
"entity_id_post_migration": (0, 1),
|
||||
},
|
||||
["ix_states_entity_id_last_updated_ts"],
|
||||
@ -143,13 +143,43 @@ def _create_engine_test(
|
||||
"event_context_id_as_binary": (0, 0),
|
||||
"event_type_id_migration": (0, 0),
|
||||
"entity_id_migration": (2, 1),
|
||||
"event_id_post_migration": (0, 0),
|
||||
"event_id_post_migration": (1, 1),
|
||||
"entity_id_post_migration": (0, 1),
|
||||
},
|
||||
["ix_states_entity_id_last_updated_ts"],
|
||||
),
|
||||
(
|
||||
38,
|
||||
{
|
||||
"state_context_id_as_binary": (0, 0),
|
||||
"event_context_id_as_binary": (0, 0),
|
||||
"event_type_id_migration": (0, 0),
|
||||
"entity_id_migration": (0, 0),
|
||||
"event_id_post_migration": (1, 1),
|
||||
"entity_id_post_migration": (0, 0),
|
||||
},
|
||||
[],
|
||||
),
|
||||
(
|
||||
43,
|
||||
{
|
||||
"state_context_id_as_binary": (0, 0),
|
||||
"event_context_id_as_binary": (0, 0),
|
||||
"event_type_id_migration": (0, 0),
|
||||
"entity_id_migration": (0, 0),
|
||||
# Schema was not bumped when the SQLite
|
||||
# table rebuild was implemented so we need
|
||||
# run event_id_post_migration up until
|
||||
# schema 44 since its the first one we can
|
||||
# be sure has the foreign key constraint was removed
|
||||
# via https://github.com/home-assistant/core/pull/120779
|
||||
"event_id_post_migration": (1, 1),
|
||||
"entity_id_post_migration": (0, 0),
|
||||
},
|
||||
[],
|
||||
),
|
||||
(
|
||||
44,
|
||||
{
|
||||
"state_context_id_as_binary": (0, 0),
|
||||
"event_context_id_as_binary": (0, 0),
|
||||
@ -266,8 +296,14 @@ async def test_data_migrator_logic(
|
||||
# the expected number of times.
|
||||
for migrator, mock in migrator_mocks.items():
|
||||
needs_migrate_calls, migrate_data_calls = expected_migrator_calls[migrator]
|
||||
assert len(mock["needs_migrate"].mock_calls) == needs_migrate_calls
|
||||
assert len(mock["migrate_data"].mock_calls) == migrate_data_calls
|
||||
assert len(mock["needs_migrate"].mock_calls) == needs_migrate_calls, (
|
||||
f"Expected {migrator} needs_migrate to be called {needs_migrate_calls} times,"
|
||||
f" got {len(mock['needs_migrate'].mock_calls)}"
|
||||
)
|
||||
assert len(mock["migrate_data"].mock_calls) == migrate_data_calls, (
|
||||
f"Expected {migrator} migrate_data to be called {migrate_data_calls} times, "
|
||||
f"got {len(mock['migrate_data'].mock_calls)}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("enable_migrate_state_context_ids", [True])
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user