fix #857

Author: dimitri-yatsenko

datajoint/condition.py

@@ -45,8 +45,9 @@ def __init__(self, restriction):
 
 def assert_join_compatibility(expr1, expr2):
     """
-    Determine if expressions expr1 and expr2 are join-compatible.  To be join-compatible, the matching attributes
-    in the two expressions must be in the primary key of one or the other expression.
+    Determine if expressions expr1 and expr2 are join-compatible.  To be join-compatible,
+    the matching attributes in the two expressions must be in the primary key of one or the
+    other expression.
     Raises an exception if not compatible.
     :param expr1: A QueryExpression object
     :param expr2: A QueryExpression object
@@ -58,10 +59,12 @@ def assert_join_compatibility(expr1, expr2):
             raise DataJointError('Object %r is not a QueryExpression and cannot be joined.' % rel)
     if not isinstance(expr1, U) and not isinstance(expr2, U):  # dj.U is always compatible
         try:
-            raise DataJointError("Cannot join query expressions on dependent attribute `%s`" % next(r for r in set(
-                expr1.heading.secondary_attributes).intersection(expr2.heading.secondary_attributes)))
+            raise DataJointError(
+                "Cannot join query expressions on dependent attribute `%s`" % next(
+                    r for r in set(expr1.heading.secondary_attributes).intersection(
+                        expr2.heading.secondary_attributes)))
         except StopIteration:
-            pass
+            pass  # all ok
 
 
 def make_condition(query_expression, condition, columns):

datajoint/expression.py

@@ -37,7 +37,7 @@ class QueryExpression:
     _restriction = None
     _restriction_attributes = None
     _left = []  # True for left joins, False for inner joins
-    _join_attributes = []
+    _original_heading = None  # heading before projections
 
     # subclasses or instantiators must provide values
     _connection = None
@@ -61,6 +61,11 @@ def heading(self):
         """ a dj.Heading object, reflects the effects of the projection operator .proj """
         return self._heading
 
+    @property
+    def original_heading(self):
+        """ a dj.Heading object reflecting the attributes before projection """
+        return self._original_heading or self.heading
+
     @property
     def restriction(self):
         """ a AndList object of restrictions applied to input to produce the result """
@@ -85,11 +90,10 @@ def from_clause(self):
         support = ('(' + src.make_sql() + ') as `_s%x`' % next(
             self._subquery_alias_count) if isinstance(src, QueryExpression) else src for src in self.support)
         clause = next(support)
-        for s, a, left in zip(support, self._join_attributes, self._left):
-            clause += '{left} JOIN {clause}{using}'.format(
+        for s, left in zip(support, self._left):
+            clause += 'NATURAL{left} JOIN {clause}'.format(
                 left=" LEFT" if left else "",
-                clause=s,
-                using="" if not a else " USING (%s)" % ",".join('`%s`' % _ for _ in a))
+                clause=s)
         return clause
 
     def where_clause(self):
@@ -241,34 +245,29 @@ def join(self, other, semantic_check=True, left=False):
             other = other()  # instantiate
         if not isinstance(other, QueryExpression):
             raise DataJointError("The argument of join must be a QueryExpression")
-        other_clash = set(other.heading.names) | set(
-            (other.heading[n].attribute_expression.strip('`') for n in other.heading.new_attributes))
-        self_clash = set(self.heading.names) | set(
-            (self.heading[n].attribute_expression for n in self.heading.new_attributes))
-        need_subquery1 = isinstance(self, Union) or any(
-            n for n in self.heading.new_attributes if (
-                    n in other_clash or self.heading[n].attribute_expression.strip('`') in other_clash))
-        need_subquery2 = (len(other.support) > 1 or
-                          isinstance(self, Union) or any(
-            n for n in other.heading.new_attributes if (
-                    n in self_clash or other.heading[n].attribute_expression.strip('`') in other_clash)))
+        if semantic_check:
+            assert_join_compatibility(self, other)
+        join_attributes = set(n for n in self.heading.names if n in other.heading.names)
+        # needs subquery if FROM class has common attributes with the other's FROM clause
+        need_subquery1 = need_subquery2 = bool(
+            (set(self.original_heading.names) & set(other.original_heading.names))
+            - join_attributes)
+        # need subquery if any of the join attributes are derived
+        need_subquery1 = need_subquery1 or any(n in self.heading.new_attributes for n in join_attributes)
+        need_subquery2 = need_subquery2 or any(n in other.heading.new_attributes for n in join_attributes)
         if need_subquery1:
             self = self.make_subquery()
         if need_subquery2:
             other = other.make_subquery()
-        if semantic_check:
-            assert_join_compatibility(self, other)
         result = QueryExpression()
         result._connection = self.connection
         result._support = self.support + other.support
-        result._join_attributes = (
-                self._join_attributes + [[a for a in self.heading.names if a in other.heading.names]] +
-                other._join_attributes)
         result._left = self._left + [left] + other._left
         result._heading = self.heading.join(other.heading)
         result._restriction = AndList(self.restriction)
         result._restriction.append(other.restriction)
-        assert len(result.support) == len(result._join_attributes) + 1 == len(result._left) + 1
+        result._original_heading = self.original_heading.join(other.original_heading)
+        assert len(result.support) == len(result._left) + 1
         return result
 
     def __add__(self, other):
@@ -371,6 +370,7 @@ def proj(self, *attributes, **named_attributes):
             need_subquery = any(name in self.restriction_attributes for name in self.heading.new_attributes)
 
         result = self.make_subquery() if need_subquery else copy.copy(self)
+        result._original_heading = result.original_heading
         result._heading = result.heading.select(
             attributes, rename_map=dict(**rename_map, **replicate_map), compute_map=compute_map)
         return result
@@ -525,7 +525,6 @@ def create(cls, arg, group, keep_all_rows=False):
         result._connection = join.connection
         result._heading = join.heading.set_primary_key(arg.primary_key)  # use left operand's primary key
         result._support = join.support
-        result._join_attributes = join._join_attributes
         result._left = join._left
         result._left_restrict = join.restriction  # WHERE clause applied before GROUP BY
         result._grouping_attributes = result.primary_key

tests/test_fetch.py

@@ -243,7 +243,7 @@ def test_fetch_format(self):
             assert_dict_equal(l1, l2, 'Primary key is not returned correctly')
 
         # test pandas with fetch1
-        k = (self.subject & 'subject_id=10').fetch1('KEY')
+        # k = (self.subject & 'subject_id=10').fetch1('KEY')
 
         # revert configuration of fetch format
         dj.config['fetch_format'] = 'array'