Skip to content

Correct migration to recorder schema 51 #156267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Nov 10, 2025
Merged

Correct migration to recorder schema 51 #156267

merged 5 commits into from
Nov 10, 2025

Conversation

@emontnemery
Copy link
Contributor

@emontnemery emontnemery commented Nov 10, 2025 •
edited
Loading

Proposed change

Correct migration to recorder schema 51, which adds unit_class to statistics meta data, by making sure we do case sensitive (actually binary) string comparison of the units:

  • Bump schema version to version 52
  • Remove migrator to version 51
  • Migrator to version 52 will branch according to database dialect:
    • If dialect is MySQL / MariaDB:
      • Reset unit_class to None for rows where only case insensitive string match matches the unit class
      • Redo setting of unit_class for rows where unit_class is None
    • If dialect is PostgreSQL / SQLite:
      • Redo setting of unit_class for rows where unit_class is None

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • I understand the code I am submitting and can explain how it works.
  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.
  • Any generated code has been carefully reviewed for correctness and compliance with project standards.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

@emontnemery emontnemery added this to the 2025.11.2 milestone Nov 10, 2025
Copilot AI review requested due to automatic review settings November 10, 2025 14:14
@emontnemery emontnemery requested a review from a team as a code owner November 10, 2025 14:14
@home-assistant
Copy link

home-assistant bot commented Nov 10, 2025

Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration (recorder) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of recorder can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign recorder Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

This comment was marked as spam.

Sign in to view

@bdraco bdraco self-requested a review November 10, 2025 15:19
MartinHjelmare
MartinHjelmare approved these changes Nov 10, 2025
View reviewed changes
homeassistant/components/recorder/migration.py
.where(
and_(
StatisticsMeta.unit_of_measurement.in_(conv.VALID_UNITS),
StatisticsMeta.unit_class.is_(None),
Copy link
Member

@MartinHjelmare MartinHjelmare Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe explain this check with a code comment as above?

@emontnemery emontnemery added the second-opinion-wanted Add this label when a reviewer needs a second opinion from another member. label Nov 10, 2025
@emontnemery
Copy link
Contributor Author

emontnemery commented Nov 10, 2025

I'd like an approving review also from @bdraco before merging

@bdraco
Copy link
Member

bdraco commented Nov 10, 2025 •
edited
Loading

Testing matrix

  • local sqlite -- verified statistics_meta had no unexpected changes
  • local mariadb
2025-11-10 10:15:41.923 WARNING (Recorder) [homeassistant.components.recorder.migration] The database is about to upgrade from schema version 48 to 52
2025-11-10 10:15:41.923 WARNING (Recorder) [homeassistant.components.recorder.migration] Upgrading recorder db schema to version 49
2025-11-10 10:15:41.923 WARNING (Recorder) [homeassistant.components.recorder.migration] Adding columns mean_type to table statistics_meta. Note: this may take several minutes on large databases and slow machines. Please be patient!
2025-11-10 10:15:41.946 WARNING (Recorder) [homeassistant.components.recorder.migration] Adding columns mean_weight to table statistics. Note: this may take several minutes on large databases and slow machines. Please be patient!
2025-11-10 10:15:42.035 WARNING (Recorder) [homeassistant.components.recorder.migration] Adding columns mean_weight to table statistics_short_term. Note: this may take several minutes on large databases and slow machines. Please be patient!
2025-11-10 10:15:42.132 WARNING (Recorder) [homeassistant.components.recorder.migration] Upgrade to version 49 done
2025-11-10 10:15:42.132 WARNING (Recorder) [homeassistant.components.recorder.migration] Upgrading recorder db schema to version 50
2025-11-10 10:15:42.153 WARNING (Recorder) [homeassistant.components.recorder.migration] Upgrade to version 50 done
2025-11-10 10:15:42.153 WARNING (Recorder) [homeassistant.components.recorder.migration] Upgrading recorder db schema to version 51
2025-11-10 10:15:42.166 WARNING (Recorder) [homeassistant.components.recorder.migration] Upgrade to version 51 done
2025-11-10 10:15:42.166 WARNING (Recorder) [homeassistant.components.recorder.migration] Upgrading recorder db schema to version 52
2025-11-10 10:15:42.166 WARNING (Recorder) [homeassistant.components.recorder.migration] Adding columns unit_class to table statistics_meta. Note: this may take several minutes on large databases and slow machines. Please be patient!
2025-11-10 10:15:42.407 WARNING (Recorder) [homeassistant.components.recorder.migration] Upgrade to version 52 done
2025-11-10 10:15:42.424 WARNING (Recorder) [homeassistant.components.recorder.util] Ended unfinished session (id=133 from 2025-02-28 16:50:10)
  • review binlog for mariadb to make sure executed statements look correct
SET unit_class = NULL
WHERE unit_of_measurement IN ('kWh', 'MWh')
  AND CAST(unit_of_measurement AS BINARY) NOT IN (
    CAST('kWh' AS BINARY),
    CAST('MWh' AS BINARY)
  );

UPDATE statistics_meta
SET unit_class = 'energy'
WHERE CAST(unit_of_measurement AS BINARY) IN (
    CAST('kWh' AS BINARY),
    CAST('MWh' AS BINARY)
  )
  AND unit_class IS NULL;
  • prod sqlite

emontnemery and others added 2 commits November 10, 2025 19:50
@emontnemery @MartinHjelmare
Apply suggestions from code review
db23476 
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
@emontnemery
Add comments
61b9872
@emontnemery emontnemery merged commit 0cf0f10 into dev Nov 10, 2025
60 checks passed
@emontnemery emontnemery deleted the recorder_fix_migration_to_schema_51 branch November 10, 2025 19:14
@github-actions github-actions bot locked and limited conversation to collaborators Nov 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@bdraco bdraco bdraco approved these changes

@MartinHjelmare MartinHjelmare MartinHjelmare approved these changes

Assignees

No one assigned

Labels

bugfix by-code-owner cherry-picked cla-signed core has-tests integration: recorder Quality Scale: internal second-opinion-wanted Add this label when a reviewer needs a second opinion from another member.

Projects

None yet

Milestone

2025.11.2

Development

Successfully merging this pull request may close these issues.

2025.11.b1 Statistics Repair False positive

5 participants

@emontnemery @bdraco @MartinHjelmare @frenck