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

TASK: Fix name of index on PersistentResource.sha1 #1569

Conversation

kdambekalns
Copy link
Member

The name IDX_35DC14F03332102A is different from what Doctrine does
auto-generate, but needs to be used due to BC reasons with existing
migrations.

See neos/neos-development-collection#2475

The name IDX_35DC14F03332102A is different from what Doctrine does
auto-generate, but needs to be used due to BC reasons with existing
migrations.

See neos/neos-development-collection#2475
@kdambekalns
Copy link
Member Author

kdambekalns commented May 3, 2019

FYI: This is for 5.3, since the used option is only available as of DBAL 2.9, which we don't allow in earlier versions.
Nonsense…

Copy link
Member

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

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

I didn't test but the approach makes sense to me and I fully trust Karsten on the index name & syntax :)

just wondering: for 6.0 we could instead provide a migration that renames (i.e. drops and re-adds) the index, no?

@albe
Copy link
Member

albe commented May 3, 2019

FYI: This is for 5.3, since the used option is only available as of DBAL 2.9, which we don't allow in earlier versions.

Wait, @Index(name="...", ..) is only available as of DBAL 2.9? No, you mean something else, right?

But what is with versions lower 5.3? They will still have the issue of missing migrations on a clean install?

@kdambekalns
Copy link
Member Author

@bwaidelich Well, yes… But we'd need to adjust all the other migrations that rely on this index name as well. The problem is solvable, but I doubt it makes sense with the name given now.

@albe Yes, I mixed that up. That was about the length, see neos/neos-development-collection#2478 Which means we could target lower branches with this one, yay!

But some of the migration differences will still be detected, whatever we do, due to mismatches between DB reporting some values and Doctrine expectations.

@albe
Copy link
Member

albe commented May 6, 2019

But some of the migration differences will still be detected, whatever we do

Iff this improves the current situation, let's get it in (lowest branch) anyway.

Copy link
Member

@albe albe left a comment

Choose a reason for hiding this comment

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

Looks good. Haven't tested on a fresh install how it behaves though.

@daniellienert daniellienert merged commit e0c298a into neos:5.3 May 9, 2019
@kdambekalns kdambekalns deleted the bugfix/neos-2475-migration-adjustments-5.3 branch May 13, 2019 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants