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.

Sign up for GitHub

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

Jump to bottom

Fix MySQL when adding primary key #363

Closed
wants to merge 2 commits into from
Closed

Fix MySQL when adding primary key #363

wants to merge 2 commits into from

Conversation

bartv2
Copy link
Contributor

@bartv2 bartv2 commented Aug 23, 2013

No description provided.

@doctrinebot
Copy link

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DBAL-586

We use Jira to track the state of pull requests and the versions they got
included in.

deeky666
deeky666 reviewed Aug 23, 2013
View reviewed changes
lib/Doctrine/DBAL/Platforms/MySqlPlatform.php
foreach ($diff->changedIndexes as $changedKey => $changedIndex) {
if ($changedIndex->isPrimary() && $changedKey != 'PRIMARY') {
$index = $diff->changedIndexes[$changedKey];
$index = new index($changedKey, $index->getColumns(), $index->isUnique(), false);
Copy link
Member

Choose a reason for hiding this comment

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

new Index (with a capital I).

@deeky666
Copy link
Member

Please also provide a test case for this issue.

@bartv2
Add primary key to 'ALTER TABLE' in MySql
dc80b8e 
This is at least needed when adding a autoincrement column
@bartv2
When changing from a non-primary index to a primary index, the droppe…
c5e5b57 
…d index isn't named PRIMARY
@beberlei
Copy link
Member

beberlei commented Sep 8, 2013

@bartv2 Can you elaborate on the problem here, why it appears for you and what your fix does?

@bartv2
Copy link
Contributor Author

bartv2 commented Sep 8, 2013

there are 2 problems:

  1. when adding a autoincrement column, you need to set the primary key in the same statement.
  2. Not sure how it happend but the original schema (not created with doctrine) had primary keys (at least it was set that way) that was not named PRIMARY

@beberlei
Copy link
Member

Merged into master and 2.4

@beberlei beberlei closed this Dec 20, 2013
@deeky666
Copy link
Member

Hmm would have liked to see a functional test for this...

@AdamWill
Copy link

AdamWill commented Jan 8, 2014

Um. Unless I'm missing something, half of this change was reverted immediately after it was committed, in a confusingly-named commit:

aedfbf2

Why was that? I think it's the reason OwnCloud still has problems with DBAL 2.4.2, though I thought all the OC patches or equivalents had now been merged.

@deeky666
Copy link
Member

deeky666 commented Jan 8, 2014

Looks like a backport problem to 2.4 branch...

@AdamWill
Copy link

AdamWill commented Jan 8, 2014

Master has the same sequence:

075c68b (change is committed)
894493b (change is reverted)

they come in immediate sequence.

@AdamWill
Copy link

AdamWill commented Jan 8, 2014

OwnCloud does still appear to need this change vs. DBAL 2.4.2. Upgrades from OC 5.0.14a to 6.0.0a (with mariadb as the database) fail with a clean 2.4.2, succeed with a 2.4.2 with this patch re-applied.

@bartv2 bartv2 deleted the fixMySQLPrimaryKey branch February 19, 2014 18:34
This was referenced Dec 6, 2015
Closed
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@bartv2 @doctrinebot @deeky666 @beberlei @AdamWill