Skip to content
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.

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

feat: Keep unchanged indices and foreign keys instead of recreating #3689

Closed
wants to merge 1 commit into from

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Sep 3, 2024

Hello,

As an alternative to switching to the standard way of handling schemas and indices, this PR avoid recreating already correct indices and foreign keys.

On foreign keys I’m less sure because I fail to see why they were recreated to start with, given that they are only removed on tables from TableSchema::FK_CHILD_TABLES, and I do not think the name of the parent table can ever change?

@come-nc
Copy link
Contributor Author

come-nc commented Sep 5, 2024

I should’ve duplicate the methods instead of renaming them, not sure how I missed that they were also used elsewhere -_-

Copy link
Collaborator

@dartcafe dartcafe left a comment

Choose a reason for hiding this comment

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

Without checking this change by detail ATM. I had a call with @AndyScherzinger yesterday. I will start a new PR to be able to get a quick solution first, which can be pushedf to the appstore and with the latest fixes.

I will link it after the first commit and decribe what my plan is. I will link it here and inside the original issue.

Would be happy to get your review and maybe some tests are possible.

@come-nc
Copy link
Contributor Author

come-nc commented Sep 9, 2024

Closing this since indices are not dropped by the repair step anymore in master 👍

@come-nc come-nc closed this Sep 9, 2024
@dartcafe dartcafe deleted the feat/keep-unchanged-indices-on-update branch October 2, 2024 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants