Added areFieldsIndexed method and Tests for it

[malarzm]

The new method is needed for doctrine/mongodb-odm#761 and checks if all queried fields are indexed.

While writing this PR I found that prefix doesn't have to be continuous but needs first field of index included (however the index is less efficient then - http://docs.mongodb.org/manual/core/index-compound/#prefixes ). Right now I'm disallowing this (it'll be a signal to developer that collection could be better indexed) but I'm not sure if it is good approach... @jmikola what do you think about it? I've added option to allow less efficient indexes, by default it's false true. I think that later in ODM we could add option to requireIndexes annotation and let developer decide

TODO:

• - add areFieldsIndexed method
• - add areFieldsIndexedForSorting method which will respect http://docs.mongodb.org/manual/core/index-compound/#sort-order

[malarzm]

Changing my approach will also simplify method :)

[jwage]

This should be:

if (!isset($index['key']) || count($index['key']) < $numFields) {

[jwage]

Curly brackets are missing.

[jmikola]

I'll try to review this over the weekend.

I've added option to allow less efficient indexes, by default it's false.

I would default to true and/or just remove that logic altogether. The only goal here is to ensure a query uses an index instead of a requiring a table scan. We can't necessarily assume that an index will be "less efficient", just because its fields are in order, as we know nothing of its selectivity or performance.

@malarzm: Why is this implemented in doctrine/mongodb if the feature to require indexes for all queries is just in ODM?

@dwieeb: Can you take a look at this and see if it meets your needs for doctrine/mongodb-odm#761?

[malarzm]

@jmikola: current implementation uses method isFieldIndexed from Collection so I thought it's good place for my method as well.

As for index efficiency I must disagree that we can't assume: According to docs Compound Indexes can be queried using prefix of an Index - if the prefix is continuous subset then index will be efficient, but if we have a "gap" in fields then index will still work but is less efficient (example from docs).

I don't think this check is necessary but developer can benefit from it - if somebody is using requireIndexes he may want to know if he fully uses his indexes (eg you decide in the middle of project that requireIndexes is good idea but you also want to be sure you've created all indexes you need - I think it's easier to find exception thrown in code than manually going through all queries project produces)

[malarzm]

@jmikola: I've changed $allowLessEfficient to be true by default (same behaviour as in Mongo) and added explanation in phpdoc so it's clear what $allowLessEffiecient mean.

And to complete my answer why the method is here: we're checking indexes on particular Collection so it should be Collection's responsibility to check it and also it'll allow people who are not using ODM to use this method :)

[imhoffd]

Judging by the unit tests, I would say this method is a good first step. Thank you, @malarzm! Good work.

[jwage]

I think this looks good. @jmikola thoughts?

Do we have any changes we need to make in the odm for this?

[imhoffd]

@jwage Yes, ODM still needs to call this method if requireIndexes is true to determine whether or not to raise an exception upon execution of a query that uses unindexed fields.

[jwage]

Cool, do we have a PR for that yet?

[malarzm]

@jwage I'll make one tomorrow, I was waiting for feedback on this first

[malarzm]

On second thought I used wrong approach. When areFieldsIndexed and areFieldsIndexedForSorting are divided there is quite a big chance that one index will include all needed fields but order will not be possible with it but second method will find another index that can sort given fields and return true anyway. These two methods needs to cooperate, I'll rework it Monday the latest.

[malarzm]

Changed approach a bit: methods do not cooperate and since they are here for developers' convenience then devs can run them both by themselves. I've only adjusted areFieldsIndexedForSorting method to allow passing $prefixPrepend array (list of fields that ran equality check in find() - those fields count when Mongo is creating index prefix).

Reasoning behind this is that "ultimate" function isQueryIndexed($find, $sort) would need to parse $find conditions, run another check for all $or clauses (they run in parallel and can use other index), consider all $and and $elemMatch operators (which right now does FieldExtractor in ODM) - I think this is out of scope of mongodb library.

[malarzm]

After all I must agree with #154 (comment) that this logic should not be here as it'll be harder to maintain it (it's way more complex than I thought at the beginning)... Closing this PR and moving all logic to ODM instead, sorry for the mess :)