Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Remove redundant event_edges.room_id and event_edges.is_state columns #12892

Open
2 of 3 tasks
richvdh opened this issue May 26, 2022 · 3 comments
Open
2 of 3 tasks

Remove redundant event_edges.room_id and event_edges.is_state columns #12892

richvdh opened this issue May 26, 2022 · 3 comments
Labels
A-Database DB stuff like queries, migrations, new/remove columns, indexes, unexpected entries in the db T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

Comments

@richvdh
Copy link
Member

richvdh commented May 26, 2022

room_id is implied by the event_id. (see also #3613)
is_state is unused since #4837.

Both are clutter and need to go away.

@squahtx squahtx added the T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. label May 27, 2022
@MadLittleMods MadLittleMods added the A-Database DB stuff like queries, migrations, new/remove columns, indexes, unexpected entries in the db label Sep 22, 2022
@MadLittleMods
Copy link
Contributor

MadLittleMods commented Sep 22, 2022

delete them altogether

At what point can we delete event_edges.is_state from our SQL queries? It seems like we have to support it forever given it's a background update to drop the rows.

I'm coming from this discussion for context: #13635 (comment)

@richvdh
Copy link
Member Author

richvdh commented Sep 23, 2022

At what point can we delete event_edges.is_state from our SQL queries? It seems like we have to support it forever given it's a background update to drop the rows.

I haven't entirely figured this out yet. I think what we need to do is add a new synchronous db migration which checks if the bg update has completed, and if not, does the update synchronously. Once that has happened, it is safe to remove the conditions from the queries, and finally to drop the columns altogether.

But we shouldn't do that until it is likely that most people have completed the bg update.

@richvdh
Copy link
Member Author

richvdh commented Sep 23, 2022

But we shouldn't do that until it is likely that most people have completed the bg update.

(That may well be true already. I haven't thought about this for a few months.)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Database DB stuff like queries, migrations, new/remove columns, indexes, unexpected entries in the db T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants