Added compound primary key support, docs and tests

[lrlopez]

This PR adds support for compound primary keys. It also includes tests and docs updates.

Closes #14

[treffynnon]

Thank you for the pull request. I have to admit I am a little wary of introducing this into Idiorm as it could have unintended consequences. For example I am not sure how this would in tandem with Paris. Have you tested it with Paris models?

[treffynnon]

I have begun the merge process in a branch: https://github.com/j4mie/idiorm/tree/feature.compound_keys

I have just tidied up a few bits and pieces.

[lrlopez]

Thank you for tiding up! I'm not currently using Paris, but I can try to run the bundled tests with the new code. Anyway, if there is any conflict it should be easy to fix...

[lrlopez]

Paris passes all the tests with the proposed changes. It was expected because the PR doesn't introduce any breaking change if you use single column primary keys.

[tag]

I like the idea, and have a table I want to use a composite PK on, but I'm a bit wary of some of this code (Looking in the PR, not the branch), for reasons that may have more to do with documentation than anything else.

Why is it that _find_many() and where_id_is() don't work with composite keys (per the included documentation)?

In some cases, it looks like it may just be the way the doc is written: "Create instances of each row in the result and map the array keys if the primary key is not compound." might be less ambiguous as "Create instances of each row in the result. If the primary key is not compound, the primary key is the array key. Otherwise, if the primary key is compound ... "

... is there no array key that would be useful? implode(',', $keys), or serialize($keys)? (Is there a use case for doing so?)

find_one() has, similar confusion in its documentation, but in that case the "limitation" should be overcome by changing the code to accept an array. (Which might already work without further changes.)

Lastly, it might be more readable to instead of having if (is_array(...)) statements everywhere, to modify ->id() to accept a bool parameter to optionally force return as an array, or to create a protected function ->id_as_array().

[lrlopez]

@tag,
Tom, you're completely right. The PR is just a proof of concept hacked in half-an-hour so that the feasibility of compound keys could be tested.

If this feature is considered for merging, I could fix all the issues so that every method could work without problems using compound keys. Simon, do you want me to do that?

[tag]

I'm a bit mixed on the idea. I love cleaner schemas and often end up in situations where I could use a composite key. However, the vision of Paris/Idiorm is 80%/20%.

The trouble is, including this feature a) makes the code more complicated, and b) requires the models to have explicit knowledge of the schema (to handle id() as an array).

... and the lack of compound key support can often be worked around by introducing a surrogate key, the way many ORMs require.

So, I guess I'm saying I'm not sure the long-term cost in complexity is worth it. (And I'm glad I don't have to make that decision!)

If we do move forward with it, I suggest spending some time first defining specific behaviors, as described above. What would the PHP array key be for returned rows? What is the recommended way for client apps to handle an array returned from ->id(), and handle internally _id_column? (And is the name semantically incorrect, if there are two columns?) Importantly, how will Paris support work?

[treffynnon]

I don't use compound keys usually and certainly not with Paris and Idiorm so this pull request is not useful to me. With that in mind if it were to be merged in I would be relying on those who want it to thoroughly test, code review and prepare an easily merge-able pull request.

[treffynnon]

Please could you guys feedback on your thoughts to do with this pull requests applicability?

[lrlopez]

I forgot to update this issue. Some weeks ago I thought of a compromise that may solve this without too much hassle. Do I still have some time to prepare a new PR showing this?

[treffynnon]

Yes, I am planning to release 1.5.0 at the end January.

[treffynnon]

See pull request #171