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(database)!: add index support for all sql dialects & index refactoring #643

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

ChrisPdgn
Copy link
Contributor

@ChrisPdgn ChrisPdgn commented Jun 21, 2023

This PR adds the full index support that was missing for MySQL, MariaDB and SQLite
In order to achieve this, refactoring was needed, which made the index endpoints more clear.

Breaking changes:

  • createIndexes() endpoint -> createIndex() endpoint, in order to define better the params
  • getDatabaseType() no longer returns 'PostgreSQL' but the sequelize dialect string ('postgres')
  • ModelOptions index types field in sequelize now has the type of Array like in mongoose

Fixes:

  • Bugs associated with indexes not being stored inside schema (field indexes + modelOptions indexes) when using the endpoints for index creation/deletion
  • 'indexes' key missing in validateModelOptions() not allowing schemas that contain modelOptions indexes to be patched

Additions:

  • Import/export indexes endpoints

Notes:

  • The index duplication bug was not encountered during testing. The created unique indexes are not duplicated, but duplication of constraints was noticed when syncing schemas with alter: true (indexes apparently don't have anything to do with it as it is a bug the exists anyway). Probably, we will wait for sequelize version 7 that fixes this bug.
    Reference: Duplicated unique constraints and indexes on sequelize.sync({ alter: true }) sequelize/sequelize#12889
  • As there is no such thing as index editing/updating, when importing indexes, if there is an already existing index with the same name as with an imported one, the imported index is ignored and the already existing one is kept

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other (please describe)

Does this PR introduce a breaking change?

  • Yes
  • No

The PR fulfills these requirements:

  • It's submitted to the main branch
  • When resolving a specific issue, it's referenced in the PR's description (e.g. fix #xxx, where "xxx" is the issue number)

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature

@ghost
Copy link

ghost commented Jun 30, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

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.

None yet

1 participant