Skip to content

Fix failing migration by conditionally deleting old constraint#33878

Merged
JordanMontgomery merged 5 commits into
mainfrom
JM-33876-failed-migration
Oct 6, 2025
Merged

Fix failing migration by conditionally deleting old constraint#33878
JordanMontgomery merged 5 commits into
mainfrom
JM-33876-failed-migration

Conversation

@JordanMontgomery
Copy link
Copy Markdown
Member

@JordanMontgomery JordanMontgomery commented Oct 6, 2025

Related issue: Resolves #33876

Modifies the migration to check for the old constraint before deleting it in case it was created on mysql 5.7

No changes file since this is an unreleased bug

Checklist for submitter

If some of the following don't apply, delete the relevant line.

  • Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements)

Testing

For unreleased bug fixes in a release candidate, one of:

  • Confirmed that the fix is not expected to adversely impact load test results

Database migrations

  • Checked table schema to confirm autoupdate
  • Checked schema for all modified table for columns that will auto-update timestamps during migration.
  • Confirmed that updating the timestamps is acceptable, and will not cause unwanted side effects.
  • Ensured the correct collation is explicitly set for character columns (COLLATE utf8mb4_unicode_ci).

@JordanMontgomery JordanMontgomery marked this pull request as ready for review October 6, 2025 17:54
@JordanMontgomery JordanMontgomery requested a review from a team as a code owner October 6, 2025 17:54
MagnusHJensen
MagnusHJensen previously approved these changes Oct 6, 2025
Copy link
Copy Markdown
Member

@MagnusHJensen MagnusHJensen left a comment

Choose a reason for hiding this comment

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

LGTM!

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 6, 2025

Codecov Report

❌ Patch coverage is 64.70588% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.15%. Comparing base (e1ca48f) to head (3d0180b).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
...ables/20250922083056_AddTableMDMAndroidProfiles.go 64.70% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #33878      +/-   ##
==========================================
+ Coverage   64.13%   64.15%   +0.01%     
==========================================
  Files        2052     2052              
  Lines      206102   206257     +155     
  Branches     6788     6788              
==========================================
+ Hits       132183   132318     +135     
- Misses      63541    63544       +3     
- Partials    10378    10395      +17     
Flag Coverage Δ
backend 65.27% <64.70%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

}

// our mysql version at the time this constraint was added did not support CHECK constraints so this may or may not exist for us
// to delete, so we create the new wider constraint above then, optionally, delete the older narrow one
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wow that's a great catch and something to keep in mind... I knew mysql 5 did not enforce those, I hadn't realized it didn't even create them.

mna
mna previously approved these changes Oct 6, 2025
@JordanMontgomery JordanMontgomery merged commit b0866dc into main Oct 6, 2025
58 of 60 checks passed
@JordanMontgomery JordanMontgomery deleted the JM-33876-failed-migration branch October 6, 2025 20:01
JordanMontgomery added a commit that referenced this pull request Oct 6, 2025
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #33876 

Modifies the migration to check for the old constraint before deleting
it in case it was created on mysql 5.7

No changes file since this is an unreleased bug

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)

## Testing

- [x] Added/updated automated tests
- [x] Where appropriate, [automated tests simulate multiple hosts and
test for host
isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing)
(updates to one hosts's records do not affect another)

- [x] QA'd all new/changed functionality manually

For unreleased bug fixes in a release candidate, one of:

- [x] Confirmed that the fix is not expected to adversely impact load
test results

## Database migrations

- [x] Checked table schema to confirm autoupdate
- [x] Checked schema for all modified table for columns that will
auto-update timestamps during migration.
- [x] Confirmed that updating the timestamps is acceptable, and will not
cause unwanted side effects.
- [x] Ensured the correct collation is explicitly set for character
columns (`COLLATE utf8mb4_unicode_ci`).
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.

Failed migration Constraint 'ck_mdm_configuration_profile_labels_apple_or_windows' does not exist

3 participants