Skip to content

Commit

Permalink
Fixed O(n) queries when adding M2M objects (#1333)
Browse files Browse the repository at this point in the history
  • Loading branch information
ddabble authored May 15, 2024
1 parent 87327ce commit e9689f8
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 4 deletions.
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ Unreleased
"Customizing the History Admin Templates" for overriding its template context (gh-1128)
- Fixed the setting ``SIMPLE_HISTORY_ENABLED = False`` not preventing M2M historical
records from being created (gh-1328)
- For history-tracked M2M fields, adding M2M objects (using ``add()`` or ``set()``)
used to cause a number of database queries that scaled linearly with the number of
objects; this has been fixed to now be a constant number of queries (gh-1333)

3.5.0 (2024-02-19)
------------------
Expand Down
11 changes: 7 additions & 4 deletions simple_history/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -687,18 +687,21 @@ def create_historical_record_m2ms(self, history_instance, instance):
m2m_history_model = self.m2m_models[field]
original_instance = history_instance.instance
through_model = getattr(original_instance, field.name).through
through_model_field_names = [f.name for f in through_model._meta.fields]
through_model_fk_field_names = [
f.name for f in through_model._meta.fields if isinstance(f, ForeignKey)
]

insert_rows = []

through_field_name = utils.get_m2m_field_name(field)
rows = through_model.objects.filter(**{through_field_name: instance})
rows = rows.select_related(*through_model_fk_field_names)
for row in rows:
insert_row = {"history": history_instance}

for through_model_field in through_model._meta.fields:
insert_row[through_model_field.name] = getattr(
row, through_model_field.name
)
for field_name in through_model_field_names:
insert_row[field_name] = getattr(row, field_name)
insert_rows.append(m2m_history_model(**insert_row))

pre_create_historical_m2m_records.send(
Expand Down
37 changes: 37 additions & 0 deletions simple_history/tests/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2376,6 +2376,43 @@ def test_bulk_add_remove(self):
historical_place = m2m_record.places.first()
self.assertEqual(historical_place.place, self.place)

def test_add_remove_set_and_clear_methods_make_expected_num_queries(self):
for num_places in (1, 2, 4):
with self.subTest(num_places=num_places):
start_pk = 100 + num_places
places = Place.objects.bulk_create(
Place(pk=pk, name=f"Place {pk}")
for pk in range(start_pk, start_pk + num_places)
)
self.assertEqual(len(places), num_places)
self.assertEqual(self.poll.places.count(), 0)

# The number of queries should stay the same, regardless of
# the number of places added or removed
with self.assertNumQueries(5):
self.poll.places.add(*places)
self.assertEqual(self.poll.places.count(), num_places)

with self.assertNumQueries(3):
self.poll.places.remove(*places)
self.assertEqual(self.poll.places.count(), 0)

with self.assertNumQueries(6):
self.poll.places.set(places)
self.assertEqual(self.poll.places.count(), num_places)

with self.assertNumQueries(4):
self.poll.places.set([])
self.assertEqual(self.poll.places.count(), 0)

with self.assertNumQueries(5):
self.poll.places.add(*places)
self.assertEqual(self.poll.places.count(), num_places)

with self.assertNumQueries(3):
self.poll.places.clear()
self.assertEqual(self.poll.places.count(), 0)

def test_m2m_relation(self):
# Ensure only the correct M2Ms are saved and returned for history objects
poll_2 = PollWithManyToMany.objects.create(question="Why", pub_date=today)
Expand Down

0 comments on commit e9689f8

Please sign in to comment.