Refactor SQL query time calculation to use Django aggregation (#720)

* Use db aggregation of python sum for total time

* Fix typo

---------

Co-authored-by: Albert Wang <[email protected]>

project/tests/test_models.py

@@ -69,16 +69,15 @@ def test_time_spent_on_sql_queries_if_has_no_related_SQLQueries(self):
 
         self.assertEqual(self.obj.time_spent_on_sql_queries, 0)
 
-    # FIXME probably a bug
     def test_time_spent_on_sql_queries_if_has_related_SQLQueries_with_no_time_taken(self):
 
         query = SQLQueryFactory()
         self.obj.queries.add(query)
 
         self.assertEqual(query.time_taken, None)
 
-        with self.assertRaises(TypeError):
-            self.obj.time_spent_on_sql_queries
+        # No exception should be raised, and the result should be 0.0
+        self.assertEqual(self.obj.time_spent_on_sql_queries, 0.0)
 
     def test_time_spent_on_sql_queries_if_has_related_SQLQueries_and_time_taken(self):
 

silk/models.py

@@ -17,6 +17,7 @@
     IntegerField,
     ManyToManyField,
     OneToOneField,
+    Sum,
     TextField,
 )
 from django.utils import timezone
@@ -124,16 +125,13 @@ def profile_table(self):
 
     @property
     def time_spent_on_sql_queries(self):
+        """"
+        Calculate the total time spent in milliseconds on SQL queries using Django aggregates.
         """
-        TODO: Perhaps there is a nicer way to do this with Django aggregates?
-        My initial thought was to perform:
-        SQLQuery.objects.filter.aggregate(Sum(F('end_time')) - Sum(F('start_time')))
-        However this feature isnt available yet, however there has been talk
-        for use of F objects within aggregates for four years
-        here: https://code.djangoproject.com/ticket/14030. It looks
-        like this will go in soon at which point this should be changed.
-        """
-        return sum(x.time_taken for x in SQLQuery.objects.filter(request=self))
+        result = SQLQuery.objects.filter(request=self).aggregate(
+            total_time=Sum('time_taken', output_field=FloatField())
+        )
+        return result['total_time'] or 0.0
 
     @property
     def headers(self):
@@ -381,4 +379,10 @@ def is_context_profile(self):
 
     @property
     def time_spent_on_sql_queries(self):
-        return sum(x.time_taken for x in self.queries.all())
+        """
+        Calculate the total time spent in milliseconds on SQL queries using Django aggregates.
+        """
+        result = self.queries.aggregate(
+            total_time=Sum('time_taken', output_field=FloatField())
+        )
+        return result['total_time'] or 0.0