- Added new option to :func:`.relationship` ``distinct_target_key``.

This enables the subquery eager loader strategy to apply a DISTINCT
to the innermost SELECT subquery, to assist in the case where
duplicate rows are generated by the innermost query which corresponds
to this relationship (there's not yet a general solution to the issue
of dupe rows within subquery eager loading, however, when joins outside
of the innermost subquery produce dupes).  When the flag
is set to ``True``, the DISTINCT is rendered unconditionally, and when
it is set to ``None``, DISTINCT is rendered if the innermost relationship
targets columns that do not comprise a full primary key.
The option defaults to False in 0.8 (e.g. off by default in all cases),
None in 0.9 (e.g. automatic by default).   Thanks to Alexander Koval
for help with this. [ticket:2836]

2 files changed, 44 insertions, 5 deletions

diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py
index 2393df26b..dbc37a4eb 100644
--- a/lib/sqlalchemy/orm/relationships.py
+++ b/lib/sqlalchemy/orm/relationships.py
@@ -103,6 +103,7 @@ class RelationshipProperty(StrategizedProperty):
         enable_typechecks=True, join_depth=None,
         comparator_factory=None,
         single_parent=False, innerjoin=False,
+        distinct_target_key=None,
         doc=None,
         active_history=False,
         cascade_backrefs=True,
@@ -372,6 +373,27 @@ class RelationshipProperty(StrategizedProperty):
           or when the reference is one-to-one or a collection that is guaranteed
           to have one or at least one entry.
 
+        :param distinct_target_key=None:
+          Indicate if a "subquery" eager load should apply the DISTINCT
+          keyword to the innermost SELECT statement.  When left as ``None``,
+          the DISTINCT keyword will be applied in those cases when the target
+          columns do not comprise the full primary key of the target table.
+          When set to ``True``, the DISTINCT keyword is applied to the innermost
+          SELECT unconditionally.
+
+          It may be desirable to set this flag to False when the DISTINCT is
+          reducing performance of the innermost subquery beyond that of what
+          duplicate innermost rows may be causing.
+
+          .. versionadded:: 0.8.3 - distinct_target_key allows the
+             subquery eager loader to apply a DISTINCT modifier to the
+             innermost SELECT.
+
+          .. versionchanged:: 0.9.0 - distinct_target_key now defaults to
+             ``None``, so that the feature enables itself automatically for
+             those cases where the innermost query targets a non-unique
+             key.
+
         :param join_depth:
           when non-``None``, an integer value indicating how many levels
           deep "eager" loaders should join on a self-referring or cyclical
@@ -621,6 +643,7 @@ class RelationshipProperty(StrategizedProperty):
         self.enable_typechecks = enable_typechecks
         self.query_class = query_class
         self.innerjoin = innerjoin
+        self.distinct_target_key = distinct_target_key
         self.doc = doc
         self.active_history = active_history
         self.join_depth = join_depth
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 6ca737c64..009bf74a4 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -692,7 +692,7 @@ class SubqueryLoader(AbstractRelationshipLoader):
             elif subq_path.contains_mapper(self.mapper):
                 return
 
-        subq_mapper, leftmost_mapper, leftmost_attr = \
+        subq_mapper, leftmost_mapper, leftmost_attr, leftmost_relationship = \
                 self._get_leftmost(subq_path)
 
         orig_query = context.attributes.get(
@@ -703,7 +703,8 @@ class SubqueryLoader(AbstractRelationshipLoader):
         # produce a subquery from it.
         left_alias = self._generate_from_original_query(
                             orig_query, leftmost_mapper,
-                            leftmost_attr, entity.mapper
+                            leftmost_attr, leftmost_relationship,
+                            entity.mapper
         )
 
         # generate another Query that will join the
@@ -752,11 +753,12 @@ class SubqueryLoader(AbstractRelationshipLoader):
             leftmost_mapper._columntoproperty[c].class_attribute
             for c in leftmost_cols
         ]
-        return subq_mapper, leftmost_mapper, leftmost_attr
+        return subq_mapper, leftmost_mapper, leftmost_attr, leftmost_prop
 
     def _generate_from_original_query(self,
             orig_query, leftmost_mapper,
-            leftmost_attr, entity_mapper
+            leftmost_attr, leftmost_relationship,
+            entity_mapper
     ):
         # reformat the original query
         # to look only for significant columns
@@ -767,8 +769,22 @@ class SubqueryLoader(AbstractRelationshipLoader):
         if not q._from_obj and entity_mapper.isa(leftmost_mapper):
             q._set_select_from([entity_mapper], False)
 
+        target_cols = q._adapt_col_list(leftmost_attr)
+
         # select from the identity columns of the outer
-        q._set_entities(q._adapt_col_list(leftmost_attr))
+        q._set_entities(target_cols)
+
+        distinct_target_key = leftmost_relationship.distinct_target_key
+
+        if distinct_target_key is True:
+            q._distinct = True
+        elif distinct_target_key is None:
+            # if target_cols refer to a non-primary key or only
+            # part of a composite primary key, set the q as distinct
+            for t in set(c.table for c in target_cols):
+                if not set(target_cols).issuperset(t.primary_key):
+                    q._distinct = True
+                    break
 
         if q._order_by is False:
             q._order_by = leftmost_mapper.order_by