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

fix(se): Include duplicate indices (w/ different names) in migrations w/o drift #4894

Closed
wants to merge 1 commit into from

Conversation

Druue
Copy link
Contributor

@Druue Druue commented May 30, 2024

Handle indices on same columns with different names without ignoring in migrations

You can find a write-up further explaining the bug here

related:

@Druue Druue added this to the 5.15.0 milestone May 30, 2024
@Druue Druue requested a review from a team as a code owner May 30, 2024 14:41
@Druue Druue requested review from Weakky and removed request for a team May 30, 2024 14:41
@Druue Druue added the PR: Bug A PR That Fixes a bug label May 30, 2024
@Druue Druue self-assigned this May 30, 2024
@Druue Druue requested a review from SevInf May 30, 2024 14:43
Copy link

codspeed-hq bot commented May 30, 2024

CodSpeed Performance Report

Merging #4894 will not alter performance

Comparing fix/dupe_indices_drift (f6f5d00) with main (36ed37c)

Summary

✅ 11 untouched benchmarks

Copy link
Contributor

WASM Query Engine file Size

Engine This PR Base branch Diff
Postgres 2.042MiB 2.042MiB 0.000B
Postgres (gzip) 813.786KiB 813.789KiB -3.000B
Mysql 2.012MiB 2.012MiB 0.000B
Mysql (gzip) 800.072KiB 800.072KiB 0.000B
Sqlite 1.914MiB 1.914MiB 0.000B
Sqlite (gzip) 762.789KiB 762.791KiB -2.000B

Copy link
Member

@SevInf SevInf left a comment

Choose a reason for hiding this comment

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

Tests do not like it, it seems

@SevInf
Copy link
Member

SevInf commented Jun 12, 2024

Closing this for now, as a fix is for thoertical bug that have not been reported yet by anyone and it turned fix is way more complicated than it is worth for now.

@SevInf SevInf closed this Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Bug A PR That Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants