Skip to content

Commit 0cd1b21

Browse files
committed
Add performance regression test for config context annotation
The test verifies that: - Main device queries do not use expensive DISTINCT operations - Tag subqueries properly use DISTINCT to prevent duplicates from issue #5387 This ensures the optimization from issue #20327 (moving .distinct() from maintaining query to tag subquery) cannot be accidentally reverted while maintaining the correctness guarantees for issues #5314 and #5387.
1 parent 5a1ec8d commit 0cd1b21

File tree

1 file changed

+54
-20
lines changed

1 file changed

+54
-20
lines changed

netbox/extras/tests/test_models.py

Lines changed: 54 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
import re
12
import tempfile
23
from pathlib import Path
34

5+
from django.db import connection
46
from django.forms import ValidationError
57
from django.test import tag, TestCase
68

@@ -458,6 +460,34 @@ def test_virtualmachine_site_context(self):
458460
vms[1].get_config_context()
459461
)
460462

463+
def test_valid_local_context_data(self):
464+
device = Device.objects.first()
465+
device.local_context_data = None
466+
device.clean()
467+
468+
device.local_context_data = {"foo": "bar"}
469+
device.clean()
470+
471+
def test_invalid_local_context_data(self):
472+
device = Device.objects.first()
473+
474+
device.local_context_data = ""
475+
with self.assertRaises(ValidationError):
476+
device.clean()
477+
478+
device.local_context_data = 0
479+
with self.assertRaises(ValidationError):
480+
device.clean()
481+
482+
device.local_context_data = False
483+
with self.assertRaises(ValidationError):
484+
device.clean()
485+
486+
device.local_context_data = 'foo'
487+
with self.assertRaises(ValidationError):
488+
device.clean()
489+
490+
@tag('regression')
461491
def test_multiple_tags_return_distinct_objects(self):
462492
"""
463493
Tagged items use a generic relationship, which results in duplicate rows being returned when queried.
@@ -495,6 +525,7 @@ def test_multiple_tags_return_distinct_objects(self):
495525
self.assertEqual(ConfigContext.objects.get_for_object(device).count(), 1)
496526
self.assertEqual(device.get_config_context(), annotated_queryset[0].get_config_context())
497527

528+
@tag('regression')
498529
def test_multiple_tags_return_distinct_objects_with_seperate_config_contexts(self):
499530
"""
500531
Tagged items use a generic relationship, which results in duplicate rows being returned when queried.
@@ -543,32 +574,35 @@ def test_multiple_tags_return_distinct_objects_with_seperate_config_contexts(sel
543574
self.assertEqual(ConfigContext.objects.get_for_object(device).count(), 2)
544575
self.assertEqual(device.get_config_context(), annotated_queryset[0].get_config_context())
545576

546-
def test_valid_local_context_data(self):
547-
device = Device.objects.first()
548-
device.local_context_data = None
549-
device.clean()
550-
551-
device.local_context_data = {"foo": "bar"}
552-
device.clean()
577+
@tag('performance', 'regression')
578+
def test_config_context_annotation_query_optimization(self):
579+
"""
580+
Regression test for issue #20327: Ensure config context annotation
581+
doesn't use expensive DISTINCT on main query.
553582
554-
def test_invalid_local_context_data(self):
583+
Verifies that DISTINCT is only used in tag subquery where needed,
584+
not on the main device query which is expensive for large datasets.
585+
"""
586+
# Use existing test device from ConfigContextTest setup
555587
device = Device.objects.first()
556588

557-
device.local_context_data = ""
558-
with self.assertRaises(ValidationError):
559-
device.clean()
589+
# connection.queries_log.clear()
560590

561-
device.local_context_data = 0
562-
with self.assertRaises(ValidationError):
563-
device.clean()
591+
# Execute annotation
592+
list(Device.objects.filter(pk=device.pk).annotate_config_context_data())
564593

565-
device.local_context_data = False
566-
with self.assertRaises(ValidationError):
567-
device.clean()
594+
device_queries = [q['sql'] for q in connection.queries_log if 'dcim_device' in q['sql']]
568595

569-
device.local_context_data = 'foo'
570-
with self.assertRaises(ValidationError):
571-
device.clean()
596+
for query in device_queries:
597+
# Main device query should NOT use DISTINCT
598+
self.assertNotIn('SELECT DISTINCT "dcim_device"', query)
599+
600+
# Tag sub-query _should_ use DISTINCT
601+
tag_distinct_pattern = re.compile(
602+
r'SELECT DISTINCT \w+\."tag_id".*FROM "extras_taggeditem" \w+ INNER JOIN "django_content_type"'
603+
)
604+
distinct_tag_pattern_found = any(tag_distinct_pattern.search(query) for query in device_queries)
605+
self.assertTrue(distinct_tag_pattern_found)
572606

573607

574608
class ConfigTemplateTest(TestCase):

0 commit comments

Comments
 (0)