rewrite query.with_polymorphic()

[Mike Bayer]

The essence of with_polymorphic() is simply to remove all polymorphic symbols from the mappers being selected, and to set the FROM clause of the query.

Right now it's a convoluted process of embedding froms and other state into each _MapperEntity and it sort of "works out", though only for one mapper and not at all for column entities.

The procedure within _join_left_to_right() is also sort of wrong, when it calls _entity_info() to get info about the "left" - that will pull up the default polymorphic attributes, and this is never corrected for the fact that it may have been adjusted via with_polymorphic(). half of the join mechanism is broken after that and we rely upon the awkward ent.corresponds_to(left) step later to work things out. perhaps this can be simplified to get the correct information up front.

the attached patch gets with_polymorphic to work a bit with columns but introduces other issues. this may need to go to 0.8, but this would be very good to further trim the internals of query.

[Mike Bayer]

so after some cleanup of _join_left_to_right, we can see if making use of _polymorphic_adapters is worth it. This patch illustrates that while we can use it, we really don't need it:

diff -r 35cba3c84ab9c36663e57049e56e59cace5aa2f7 lib/sqlalchemy/orm/query.py
--- a/lib/sqlalchemy/orm/query.py   Thu Feb 16 19:29:00 2012 -0500
+++ b/lib/sqlalchemy/orm/query.py   Thu Feb 16 20:02:04 2012 -0500
@@ -1791,6 +1791,7 @@
     def _join_to_left(self, left, right, right_is_aliased, onclause, outerjoin):
         left_mapper, left_selectable, left_is_aliased = _entity_info(left)

+
         # this is an overly broad assumption here, but there's a 
         # very wide variety of situations where we rely upon orm.join's
         # adaption to glue clauses together, with joined-table inheritance's
@@ -1801,6 +1802,12 @@
         # whole thing the way we are here).
         join_to_left = not right_is_aliased and not left_is_aliased

+        if left_mapper in self._polymorphic_adapters:
+            left_selectable = self._polymorphic_adapters[left_mapper](left_mapper).selectable
+            magic_flag = True
+        else:
+            magic_flag = False
+
         if self._from_obj and left_selectable is not None:
             replace_clause_index, clause = sql_util.find_join_source(
                                                     self._from_obj, 
@@ -1815,7 +1822,8 @@
                 # An exception case where adaption to the left edge is not
                 # desirable.  See above note on join_to_left.
                 if join_to_left and isinstance(clause, expression.Join) and \
-                    sql_util.clause_is_present(left_selectable, clause):
+                    sql_util.clause_is_present(left_selectable, clause) and \
+                    not magic_flag:
                     join_to_left = False

                 try:

Above, we use the very comprehensive "find_join_source" in any case to match up the left selectable with _from_obj in the usual case that the left_mapper is in _polymorphic_adapters. This works more generically including when _polymorphic_adapters is empty. Also, some tests such as test_query_subclass_join_to_base_relationship in test_polymorphic_rel don't use with_polymorphic but still need to fall into the "ent.corresponds_to()" part, so we can't change that either. So there's not much point in looking into _polymorphic_adapters here.

[Mike Bayer]

OK this will actually be very easy, the hardest part will be dealing with the old with_polymorphic() code either as just cruft that has to be maintained, or expressing it in terms of the new system which might be tricky. But the new idea is totally easy and can even go in 0.7 as it doesn't affect much. A free-standing "polymorphic" object, approximated here:

        def test_aliased_custom_polymorphic(self):
            sess = create_session()
            pm = class_mapper(Person)
            mappers, selectable = pm._with_polymorphic_args([Manager](Engineer,), False)
            pa = aliased(Person, selectable)
            ea = aliased(Engineer, selectable)
            ma = aliased(Manager, selectable)

            pa._special_mapper_thing = mappers
            def go():
                eq_(sess.query(pa)
                        .filter(ea.primary_language == 'java').all(),
                    self._emps_wo_relationships_fixture()[0:1](0:1))
            self.assert_sql_count(testing.db, go, 1)

            ma = aliased(Manager, selectable)
            eq_(
                sess.query(pa.name, ea.primary_language, ma.manager_name).\
                    filter(or_(ea.primary_language=='java', ma.manager_name=='dogbert')).\
                    order_by(pa.type).all(),
                [                   (u'dilbert', u'java', None),
                    (u'dogbert', None, u'dogbert'), 
                ](
)
            )

An adjustment in query.py to look for _special_mapper_thing and that test passes for all styles of with_polymorphic.

Also it appears that all the with_polymorphic() tests currently "pass", but don't do the right thing at all, as regardless of what the "polymorphic" selectable is, it drops down to the hand-constructed join. This whole thing with the custom aliased unions/joins in a non-concrete setup is total cruft at the moment, since there's no real benefit to it in any case.

[Mike Bayer]

patch is attached. The changes to Query are minimal, the existing Aliased system is used normally, with a few tweaks, to achieve the result. Most of the patch is a rework of test_polymorphic_rel.py to break out the fixture into a new suite. AFAICT you can now make any entity into a with_polymorphic() of any kind and use it as freely as any other entity.

[Mike Bayer]

this could be in 0.7.6 but would be nicer as a fresh start in 0.8. Needs docs, the existing with_polymorphic() section should be thrown out entirely. Perhaps moving the inheritance docs to declarative could happen here too.

[Mike Bayer]

this is in 0.8 branch at cb3913a186a01d9425e0ba97de89aa6d7d64ab96

[Mike Bayer]

Removing milestone: 0.8.0b1 (automated comment)