[patch] Problem resolving primary key in some kwarg database queries

[freakboy@…]

This patch is provided against magic_removal. The same logic should apply equally well on the trunk.

Queries involving primary keys over a relation currently exhibit some problems. The existing logic in parse_lookup works out if the final query clause is 'pk', and if so, replaces 'pk' with '(primary key)__exact'. This is fine, except that the primary key is determined from the Options object provided to parse_lookup, which is he Options object relating to the main table. Some additional context data is added if the field is a related field.

However, if a custom primary key is in use, and/or the pk query is several levels deep, the primary key is incorrectly resolved, as a different options object is required. If a one-to-one relationship is being used, the extra context is not required.

To overcome these problems, I've done a rework of parse_lookup. The following are the most notable features of the change:

1. The algorithm is now recursive, rather than iterative. This simplifies the internal logic, and contains less repetition of code (especially with isnull and exact queries). The algorithm is also documented much more extensively than previously.
2. Primary key lookup behaviour is now determined on the correct Options object
3. Joins now utilize a 'Sorted Map' data structure, to ensure that joins are applied to the final query in the order that they are constructed.
4. One-many queries (i.e., reverse lookup over Foreign Keys) are now possible. As well as an Article finding its (single) Reporter, a Reporter can query over all their (many) Articles.
5. Error messages produced are a little more explicit as to the source of the problem with a kwarg query.

The fix comes as two patches; one to implement the parse_lookup changes, and one to update unit tests. The two patches are almost completely independent.

If you apply the parse_lookup patch without the unit test patch, all existing unit tests will pass, with the exception of two: custom_columns, Line 17 and many_to_one, Line 64. These tests fail due to minor changes in the text of error messages. The same exception is thrown for the same reason, but the error text has changed (as a result of point 4 above). This provides partial proof that the parse_lookup algorithm changes don't change behaviour.

If you apply the unit test patch without the parse_lookup patch, there are 17 failures. 8 of these failures are not really failures in the existing algorithm:

• 2 failures are due to error text changes
• 6 failures are due to one-to-many queries not supported in the existing parse_lookup.

However, the remaining 9 failures highlight problems in the existing algorithm:

• 2 failures are due to custom primary key selection
• 2 failures are due to primary key selection in many-to-one relations
• 5 failures highlight problems in one_to_one primary key selection

If you apply both patches, all tests pass.

As an aside, this patch has also adds caching to get_all_related_many_to_many_objects() on Option objects. It uses the same scheme that is used by get_all_related_objects().

[freakboy@…]

Patch for primary key lookup in parse_lookup

[freakboy@…]

Unit tests to validate patch for primary key lookup in parse_lookup

[Adrian Holovaty]

Wow, this is great stuff! Thank you!

[Adrian Holovaty]

(In [1855]) magic-removal: Changed get_all_related_many_to_many_objects to use caching. Refs #1186. Thanks, Russ

[Adrian Holovaty]

(In [1856]) magic-removal: Fixed #1186 -- Fixed problem resolving primary key in some 'pk' database queries. Also lightly refactored query-parsing code. Thanks, Russ