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

Allow overriding implicit indexes #769

Merged
merged 1 commit into from
Jan 9, 2015

Conversation

deeky666
Copy link
Member

@deeky666 deeky666 commented Jan 8, 2015

This is a follow-up patch for the change introduced in #764.
It makes the Table class act more intelligent when it comes to preferring explicit over implicit indexes.
Currently it is necessary to construct a Table instance in a specific order to not have unnecessary duplicate indexes because of foreign keys. If you first add a foreign key and afterwards and explicit index that is fulfilling the implicit one, both indexes are stored. Instead it is more intelligent to replace the implicit by the explicit one if and only if the explicit one fulfills the implicit one.
This should also fix possible issues with how ORM's schema tool constructs a schema. See here where for a OneToOne relation on a primary key an additional regular index for the foreign key is created.

@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-1108

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

@stof
Copy link
Member

stof commented Jan 8, 2015

What about getIndex ? Should it take implicit indexes into account ? If it does not, it might be a BC break

@deeky666
Copy link
Member Author

deeky666 commented Jan 8, 2015

@stof implicit indexes are also stored in the regular index array. So this is no problem.

@Ocramius Ocramius self-assigned this Jan 9, 2015
Ocramius added a commit that referenced this pull request Jan 9, 2015
@Ocramius Ocramius merged commit 32fae86 into doctrine:master Jan 9, 2015
Ocramius added a commit that referenced this pull request Jan 9, 2015
@Ocramius
Copy link
Member

Ocramius commented Jan 9, 2015

Merged, thanks @deeky666!

master: 32fae86
2.5: 4067c72

@webdevilopers
Copy link

👍

I can confirm this works for indexes on Table now. I was trying the same for uniqueConstraints:

/**
 * @ORM\Entity(repositoryClass="Application\Sonata\UserBundle\Entity\UserRepository")
 * @ORM\Table(name="users",
 *   indexes={
 *     @ORM\Index(name="compartment_idx", columns={"compartment_id"}),
 *   },
 *   uniqueConstraints={
 *     @ORM\UniqueConstraint(name="username_canonical_unique", columns={"username_canonical"})
 *   }
 * )
 */

but running doctrine:migrations:diff creates the same unique index twice:

class Version20150114110814 extends AbstractMigration
{
    public function up(Schema $schema)
    {
        $this->addSql('CREATE INDEX compartment_idx ON users (compartment_id)');
        $this->addSql('CREATE UNIQUE INDEX UNIQ_1483A5E992FC23A8 ON users (username_canonical)');
        $this->addSql('CREATE UNIQUE INDEX username_canonical_unique ON users (username_canonical)');
    }
}

@deeky666
Copy link
Member Author

The unique index UNIQ_1483A5E992FC23A8 is probably created by a column mapping with option unique=true. As this one is auto-generated implicitly by ORM's schema tool, we might consider filtering those out if a fulfilling custom one is present.
@Ocramius what do you think? Btw besides creating a unique index, what does @Column(unique=true) else do?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants