relations to a polymorphic entities are broken starting SA 0.3.4

Issue #439 resolved
Former user created an issue

Retrieving an object from a polymorphic relation does not generate a correct query (syntax is correct, but not the sense). Using {{{session.query(PolymorphicMapper).get(x.polymorphic_object_id)}}} in place of {{{x.polymorphic_object}}} (property) generates the correct SQL query.

In the attached script, using the property generates this:

SELECT employee_join.person_id AS employee_join_person_id, employee_join.longer_status AS employee_join_longer_status, employee_join.status AS employee_join_status, employee_join.name AS employee_join_name, employee_join.type AS employee_join_type
FROM
(
    SELECT managers.person_id AS person_id, managers.longer_status AS longer_status, CAST(NULL AS VARCHAR(30)) AS status, people.name AS name, 'manager' AS type
    FROM people JOIN managers ON people.person_id = managers.person_id
    UNION ALL
    SELECT engineers.person_id AS person_id, CAST(NULL AS VARCHAR(70)) AS longer_status, engineers.status AS status, people.name AS name, 'engineer' AS type
    FROM people JOIN engineers ON people.person_id = engineers.person_id
) AS employee_join, people
WHERE people.person_id = ? ORDER BY employee_join.oid

Using a Query.get() on the mapper generates the correct SQL:

SELECT employee_join.person_id AS employee_join_person_id, employee_join.longer_status AS employee_join_longer_status, employee_join.status AS employee_join_status, employee_join.name AS employee_join_name, employee_join.type AS employee_join_type
FROM
(
    SELECT managers.person_id AS person_id, managers.longer_status AS longer_status, CAST(NULL AS VARCHAR(30)) AS status, people.name AS name, 'manager' AS type
    FROM people JOIN managers ON people.person_id = managers.person_id
    UNION ALL
    SELECT engineers.person_id AS person_id, CAST(NULL AS VARCHAR(70)) AS longer_status, engineers.status AS status, people.name AS name, 'engineer' AS type
    FROM people JOIN engineers ON people.person_id = engineers.person_id
) AS employee_join
WHERE employee_join.person_id = ? ORDER BY employee_join.oid

I tried with defining the relations in 3 ways:

car_mapper = mapper(Car, cars, properties= {'employee':relation(person_mapper)})
car_mapper = mapper(Car, cars, properties= {'employee':relation(person_mapper,primaryjoin=cars.c.owner==employee_join.c.person_id)})
car_mapper = mapper(Car, cars, properties= {'employee':relation(person_mapper,primaryjoin=cars.c.owner==people.c.person_id)})

The 3rd one doesn't change anything, the 2nd one would probably help, but starting 0.3.4, it produces:

''sqlalchemy.exceptions.ArgumentError: In relationship 'employee' between mappers 'Mapper|Car|cars' and 'Mapper|Person|people', primary and secondary join conditions must not include columns from the polymorphic 'select_table' argument as of SA release 0.3.4. Construct join conditions using the base tables of the related mappers.''

Comments (4)

  1. Mike Bayer repo owner

    thanks for the very clear test case, it made fixing this problem a snap. the test case went straight into the unit tests.

    the conditions for this case were:

    • the relationship is a many-to-one (or one-to-one with the FK on the parent table)
    • the polymorphic union does not contain the base table as one of its selectables

    changeset:2250 addresses this by applying a more liberal adaption of the "primaryjoin" condition's columns to properly match the polymorphic union, i.e. using column name. a detailed description is listed in the comments of the new unit test.

    The new policy of not allowing primaryjoin conditions to the polymorphic selectable is so that we have better information about how the tables join to each other, so i think that has to stay. its less complex to adapt the primaryjoin from the base tables up to the big UNION clause than it was to adapt it from the UNION into a more rudimentary one.

    If we encounter more conditions like this one, where there is truly no way to adapt the primaryjoin to the polymorphic selectable, I will add another flag to be used in absolute last resorts only called "polymorphic_primaryjoin" where you can specify your join.

    i would like to leave 0.3.4 out at least until the weekend to gather more issues that may have been introduced, let me know if thats OK, else I can create a new release.

  2. Former user Account Deleted

    Thanks zzzeek for your very fast reaction. Patching SA using #2250 works like a charm with our application now.

    Just some comments: * As you may see in our polymorphic_union (http://www.sqlalchemy.org/trac/attachment/ticket/439/439.py#L48) we don't have table "people" in it, for the simple reason than in our concepts we have a super class (here people) that is "abstract", in other words, we have never ''Person'' objects but this one is needed to share common functionalities (+ pattern Template Method). SA should know that ''people'' is abstract to: * omit the unneeded: {{{polymorphic_identity='person'}}} * prevent the creation of Person objects * be able to automagically create the polymorphic_union * It should be greet to have several level of polymorphism, let's have:

    person <-- employee <-- manager
                        <-- engineer <-- civil_engineer
                                     <-- mechanical_engineer
           <-- contact
    

    This would automagically creates the polymorphic person_union based on table contact and polymorphic employee_union, itself based on manager and polymorphic engineer_union...

    Thanks for all!

  3. Mike Bayer repo owner

    the previous fix was not that great since it relied upon keynames. ive checked in the polymorphic relation branch in changeset:2270 which uses a way more accurate method of figuring out the joins, by calculating which columns are "equivalent" when searching in the polymorphic union.

  4. Log in to comment