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

Remove index #1054

Merged
merged 2 commits into from
Dec 15, 2020
Merged

Remove index #1054

merged 2 commits into from
Dec 15, 2020

Conversation

Oxiang
Copy link
Contributor

@Oxiang Oxiang commented Dec 14, 2020

Problem

Now that the production database uses urls_combined_weighted_search_idx, urls_weighted_search_idx is obsolete and can be deleted.

The previous db migration script that was meant to create urls_combined_weighted_search_idx is unused as sequelize handles the index creation. The script can be remove as well.

Solution

  • New db_migration script to delete urls_weighted_search_idx index
  • Remove 20201202_add_combined_search_index migration script from codebase

- sequelize handled the creation of urls_combined_weighted_search_idx
@Oxiang Oxiang requested review from LoneRifle and yong-jie December 14, 2020 06:55
Copy link
Contributor

@LoneRifle LoneRifle left a comment

Choose a reason for hiding this comment

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

we don't usually remove scripts for migrations. lgtm otherwise.

@Oxiang
Copy link
Contributor Author

Oxiang commented Dec 14, 2020

we don't usually remove scripts for migrations. lgtm otherwise.

In this case we did not use the migration script at all, would leaving it in cause confusion?

@LoneRifle
Copy link
Contributor

Ah, that's true. let me change it.

@yong-jie yong-jie merged commit 004e833 into develop Dec 15, 2020
@yong-jie yong-jie deleted the remove-index branch December 15, 2020 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants