-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Support for Partial Indexes for PostgreSql and Sqlite #1027
Conversation
Hello, thank you for creating this pull request. I have automatically opened an issue http://www.doctrine-project.org/jira/browse/DDC-3117 We use Jira to track the state of pull requests and the versions they got |
@PowerKiKi since this seems to be a platform-specific feature, what about introducing it in an "options" key of the index mappings (like tables do)? |
While this pull-request is clearly about PostgreSql, partial indexes are also possible on Sqlite and Mssql. Does that still falls in "platform-specific" definition ? But most importantly my choice was to be consistent with what existed in Doctrine 1. However, since I don't have a good overview of the Doctrine way of doing things, I am open to discussion... Wouldn't introducing an array of options for only a single possible option be slightly cumbersome to use ? |
@PowerKiKi yes, but it would still be an indicator of something "custom" going on. We can explicitly support only what is being supported by all (supported) SQL RDBMSs out there, that's why I suggested making it a stashed-away "option" |
Fair enough. Since it may impact all platform-specific things for indexes |
@deeky666 maybe (if he has time) Marco Pivetta On 14 July 2014 14:58, PowerKiKi [email protected] wrote:
|
@Ocramius @PowerKiKi sorry for the delay on this. I am tempting to agree with @Ocramius because of the given reasons. Also introducing more and more parameters for each platform specific feature is really bloating the API. I would also stick to using an options array here as being done for other specific features, too. The only "problem" I see here is that there already is a |
Could we have Btw, I don't see any mention of |
Support for Partial Indexes was available in Doctrine 1 following http://www.doctrine-project.org/jira/browse/DC-82. This commit reintroduce support for Doctrine 2. We use the same syntax with an optionnal "where" attribute for Index and UniqueConstraint.
This coherent with what is done for Table. All platform specific things are grouped into an options array. Eventually flags should be migrated into options as well.
@deeky666, @Ocramius, I moved One future thing to do would be to move Anyway I believe this PR is ready to be merged (as well as its dependency: doctrine/dbal#600) |
@Ocramius, @deeky666, a while back we merged doctrine/dbal#600, but this PR here seems to have been forgotten and without it the first PR is useless. Would you mind having a look at it one more time ? PS: on a related note, I also opened a new PR, doctrine/dbal#716, to be able to use partial indexes with "complex" conditions |
@PowerKiKi thanks for poking again: merging! |
Support for Partial Indexes for PostgreSql and Sqlite
Support for Partial Indexes was available in Doctrine 1 following
http://www.doctrine-project.org/jira/browse/DC-82. This commit
reintroduce support for Doctrine 2. We use the same syntax with an
optionnal "where" attribute for Index and UniqueConstraint.
It is unit-tests covered and documented in manual. This Pull Request depends on doctrine/dbal#600. So they should both be merged or rejected together.
Thanks for your time !