Merge pull request #963 from jverswijver/fetch_order_by_bug

Fix fetch order by bug when using DISTINCT

Author: dimitri-yatsenko

CHANGELOG.md

@@ -6,6 +6,7 @@
 * Bugfix - Replace use of numpy aliases of built-in types with built-in type. (#938) PR #939
 * Bugfix - `ExternalTable.delete` should not remove row on error (#953) PR #956
 * Bugfix - Fix error handling of remove_object function in `s3.py` (#952) PR #955
+* Bugfix - Fix regression issue with `DISTINCT` clause and `GROUP_BY` (#914) PR #963
 * Bugfix - Fix sql code generation to comply with sql mode `ONLY_FULL_GROUP_BY` (#916) PR #965
 * Bugfix - Fix count for left-joined `QueryExpressions` (#951) PR #966
 * Bugfix - Fix assertion error when performing a union into a join (#930) PR #967

datajoint/expression.py

@@ -44,6 +44,9 @@ class QueryExpression:
     _heading = None
     _support = None
 
+    # If the query will be using distinct
+    _distinct = False
+
     @property
     def connection(self):
         """ a dj.Connection object """
@@ -106,9 +109,8 @@ def make_sql(self, fields=None):
         Make the SQL SELECT statement.
         :param fields: used to explicitly set the select attributes
         """
-        distinct = self.heading.names == self.primary_key
         return 'SELECT {distinct}{fields} FROM {from_}{where}'.format(
-            distinct="DISTINCT " if distinct else "",
+            distinct="DISTINCT " if self._distinct else "",
             fields=self.heading.as_sql(fields or self.heading.names),
             from_=self.from_clause(), where=self.where_clause())
 
@@ -722,6 +724,7 @@ def __and__(self, other):
         if not isinstance(other, QueryExpression):
             raise DataJointError('Set U can only be restricted with a QueryExpression.')
         result = copy.copy(other)
+        result._distinct = True
         result._heading = result.heading.set_primary_key(self.primary_key)
         result = result.proj()
         return result

docs-parts/intro/Releases_lang1.rst

@@ -8,6 +8,7 @@
 * Bugfix - Fix sql code generation to comply with sql mode ``ONLY_FULL_GROUP_BY`` (#916) PR #965
 * Bugfix - Fix count for left-joined ``QueryExpressions`` (#951) PR #966
 * Bugfix - Fix assertion error when performing a union into a join (#930) PR #967
+* Bugfix - Fix regression issue with `DISTINCT` clause and `GROUP_BY` (#914) PR #963
 
 0.13.2 -- May 7, 2021
 ----------------------

tests/schema.py

@@ -439,3 +439,13 @@ class SessionDateA(dj.Lookup):
         ('mouse1', '2020-12-03'),
         ('mouse1', '2020-12-04')
     ]
+
+
+@schema
+class Stimulus(dj.Lookup):
+    definition = """
+    id: int
+    ---
+    contrast: int
+    brightness: int
+    """

tests/test_fetch.py

@@ -6,6 +6,7 @@
 import pandas
 import warnings
 from . import schema
+from .schema import Parent, Stimulus
 import datajoint as dj
 import os
 
@@ -287,3 +288,34 @@ def test_query_caching(self):
 
         # reset cache directory state (will fail if purge was unsuccessful)
         os.rmdir(os.path.expanduser('~/dj_query_cache'))
+
+    def test_fetch_group_by(self):
+        # https://github.com/datajoint/datajoint-python/issues/914
+
+        assert Parent().fetch('KEY', order_by='name') == [{'parent_id': 1}]
+
+    def test_dj_u_distinct(self):
+        # Test developed to see if removing DISTINCT from the select statement
+        # generation breakes the dj.U universal set imlementation
+
+        # Contents to be inserted
+        contents = [
+            (1,2,3),
+            (2,2,3),
+            (3,3,2),
+            (4,5,5)
+        ]
+        Stimulus.insert(contents)
+
+        # Query the whole table
+        test_query = Stimulus()
+
+        # Use dj.U to create a list of unique contrast and brightness combinations
+        result = dj.U('contrast', 'brightness') & test_query
+        expected_result = [{'contrast': 2, 'brightness': 3},
+                           {'contrast': 3, 'brightness': 2},
+                           {'contrast': 5, 'brightness': 5}]
+
+        fetched_result = result.fetch(as_dict=True, order_by=('contrast', 'brightness'))
+        Stimulus.delete_quick()
+        assert fetched_result == expected_result