Skip to content

[management] fix index creation if exist on mysql #4150

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

Merged
merged 8 commits into from
Jul 16, 2025

Conversation

pascal-fischer
Copy link
Collaborator

@pascal-fischer pascal-fischer commented Jul 15, 2025

Describe your changes

Issue ticket number and link

#4148

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • Extended the README / documentation, if necessary

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

@Copilot Copilot AI review requested due to automatic review settings July 15, 2025 13:27
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the CreateIndexIfNotExists migration helper by adding explicit existence checks for MySQL, PostgreSQL, and SQLite, and introduces tests that run against in-memory SQLite and real MySQL/Postgres containers.

  • Add dialect-specific queries in CreateIndexIfNotExists to skip index creation if it already exists.
  • Extend setupDatabase to spin up MySQL/Postgres test containers based on NETBIRD_STORE_ENGINE.
  • Introduce TestCreateIndex and TestCreateIndexIfExists to verify index creation and idempotency.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
management/server/migration/migration.go Added INFORMATION_SCHEMA, pg_indexes, and sqlite_master checks before creating an index.
management/server/migration/migration_test.go Extended setupDatabase to support MySQL/Postgres containers and added new tests for index creation.
Comments suppressed due to low confidence (1)

management/server/migration/migration_test.go:303

  • [nitpick] The test name TestCreateIndexIfExists is ambiguous about its intent. Consider renaming it to TestCreateIndexIdempotent or TestCreateIndexNotRecreated to better convey that it verifies idempotent index creation.
func TestCreateIndexIfExists(t *testing.T) {

Copy link

@pascal-fischer pascal-fischer merged commit 4f74509 into main Jul 16, 2025
53 of 56 checks passed
@pascal-fischer pascal-fischer deleted the fix/index-creation-on-mysql branch July 16, 2025 13:07
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.

2 participants