From be72e841f58281399202197c6fba05dbe05be6d9 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 3 Aug 2026 12:49:54 -0400 Subject: [PATCH] Closes #22837: Omit implicit pagination on GraphQL to-one prefetches --- netbox/netbox/graphql/pagination.py | 14 +++++- netbox/netbox/tests/test_graphql.py | 76 +++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/netbox/netbox/graphql/pagination.py b/netbox/netbox/graphql/pagination.py index abc62ac6b..984e41d01 100644 --- a/netbox/netbox/graphql/pagination.py +++ b/netbox/netbox/graphql/pagination.py @@ -52,8 +52,20 @@ def apply_pagination( # Enforce MAX_PAGE_SIZE on the pagination limit max_page_size = get_config().MAX_PAGE_SIZE if max_page_size: + # A limit is meaningless for a field which returns at most one object, and synthesizing one for a + # prefetched to-one relation is actively harmful. strawberry-django deliberately leaves `pagination` + # as None there so that the prefetch remains a plain `WHERE id IN (...)` query; making it non-None + # switches the prefetch to a window function partitioned by the parent ID. Every partition then + # holds exactly one row, so ROW_NUMBER() is 1 throughout and the row number filter discards nothing, + # causing the join back to the parent table to return every row which shares the related object. + # See strawberry-graphql/strawberry-django#719. + returns_single_object = not (self.is_list or self.is_paginated or self.is_connection) + if pagination is None: - pagination = OffsetPaginationInput(limit=max_page_size) + # Note that `pagination` is never None for a single-object field unless it is a prefetched + # relation: strawberry-django populates it with an implicit limit of its own beforehand. + if not returns_single_object: + pagination = OffsetPaginationInput(limit=max_page_size) elif pagination.limit in (None, UNSET) or pagination.limit > max_page_size: pagination.limit = max_page_size elif pagination.limit <= 0: diff --git a/netbox/netbox/tests/test_graphql.py b/netbox/netbox/tests/test_graphql.py index 5fe5b8468..46f06dd6e 100644 --- a/netbox/netbox/tests/test_graphql.py +++ b/netbox/netbox/tests/test_graphql.py @@ -627,6 +627,82 @@ class GraphQLAPITestCase(APITestCase): self.assertNotIn('errors', data) self.assertEqual(len(data['data']['site_list']), 2) + def test_to_one_relation_prefetch(self): + """ + A prefetched to-one relation should be fetched with a plain `WHERE id IN (...)` query, rather than + with a window function partitioned by the parent ID (which returns every row sharing the related + object, regardless of the requested page size). + """ + self.add_permissions('dcim.view_device', 'dcim.view_site') + url = reverse('graphql') + + site = Site.objects.first() + manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1') + device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Device Type 1', slug='device-type-1') + role = DeviceRole.objects.create(name='Device Role 1', slug='device-role-1') + Device.objects.bulk_create([ + Device(name=f'Device {i}', site=site, device_type=device_type, role=role) + for i in range(1, 21) + ]) + + # Request two of the twenty devices at the site + query = """ + { + device_list(pagination: {limit: 2}) { + name + site { name } + } + } + """ + with CaptureQueriesContext(connection) as ctx: + response = self.client.post(url, data={'query': query}, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_200_OK) + data = json.loads(response.content) + self.assertNotIn('errors', data) + self.assertEqual(len(data['data']['device_list']), 2) + self.assertEqual(data['data']['device_list'][0]['site']['name'], site.name) + + # The site should have been fetched by exactly one query. (Asserting that it exists keeps the + # assertions below from silently passing if the site is ever fetched some other way.) + site_queries = [q['sql'] for q in ctx.captured_queries if 'FROM "dcim_site"' in q['sql']] + self.assertEqual(len(site_queries), 1, msg=f'Expected one query against dcim_site, got {site_queries}') + + # That query should not apply window pagination, nor join back to the devices table (which would + # return one row per device at the site) + self.assertNotIn('ROW_NUMBER', site_queries[0]) + self.assertNotIn('dcim_device', site_queries[0]) + + @override_settings(MAX_PAGE_SIZE=3) + def test_max_page_size_nested_list(self): + """ + MAX_PAGE_SIZE should still be enforced on a nested list relation. + """ + self.add_permissions('dcim.view_device', 'dcim.view_site') + url = reverse('graphql') + + site = Site.objects.first() + manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1') + device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Device Type 1', slug='device-type-1') + role = DeviceRole.objects.create(name='Device Role 1', slug='device-role-1') + Device.objects.bulk_create([ + Device(name=f'Device {i}', site=site, device_type=device_type, role=role) + for i in range(1, 6) + ]) + + query = """ + { + site_list(pagination: {limit: 1}) { + name + devices { name } + } + } + """ + response = self.client.post(url, data={'query': query}, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_200_OK) + data = json.loads(response.content) + self.assertNotIn('errors', data) + self.assertEqual(len(data['data']['site_list'][0]['devices']), 3) + def test_pagination_conflict(self): url = reverse('graphql') query = """