Do not TRIM() parentheses around partial indexes conditions #716
Conversation
|
Hello, thank you for creating this pull request. I have automatically opened an issue http://www.doctrine-project.org/jira/browse/DBAL-1033 We use Jira to track the state of pull requests and the versions they got |
PowerKiKi
changed the title
|
Looks like partial index conditions could cause a lot of headaches for the comparator. Requiring the user to know how the underlying platform returns the condition while introspecting the schema is PITA :( Not mentioning portability... |
|
You are right that it is PITA. I thought we could somehow show a warning/message with what is expected when using CLI tools. Maybe with the As for portability I don't think it is such a big issue. Things in May I suggest to merge this PR as is, because the current implementation is flawed and should be fixed before being released to public ? And maybe in a second step introduce some sort of help for the expected syntax ? (Also note that the Travis was already failing in previous, non-related commit, this PR is not failing) |
|
I think this makes sense, but a few notes here:
|
|
fair points, I'll have a look at that ... |
Because PostgreSQL will return the expression with a lot of
parentheses we cannot TRIM() them easily. It is easier and more
correct to adapt to what PostgreSQL returns. That means the declaration
of partial indexes must be updated as follow:
Before:
``options={"where": "other_id IS NULL"}``
After:
``options={"where": "(other_id IS NULL)"}``
And fore more complex conditions, that would be:
``options={"where": "(((id IS NOT NULL) AND (other_id IS NULL)) AND (name IS NULL))"}``
PowerKiKi
force-pushed
the
bug-partial-indexes-with-multiple-conditions
branch
from
dfd62b6 to
853850c
Compare
|
Hey @Ocramius, I completed with functional tests, rebased on master and documented in doctrine/orm#1232. IMHO it's ready to be merged. |
|
Merged manually with some additional assertions (revealed the test case was wrong). @PowerKiKi thx! |
Because PostgreSQL will return the expression with a lot of
parentheses we cannot TRIM() them easily. It is easier and more
correct to adapt to what PostgreSQL returns. That means the declaration
of partial indexes must be updated as follow:
Before:
options={"where": "other_id IS NULL"}After:
options={"where": "(other_id IS NULL)"}And fore more complexe conditions, that would be:
options={"where": "(((id IS NOT NULL) AND (other_id IS NULL)) AND (name IS NULL))"}