Skip to content

Commit bb9743c

Browse files
committed
fix(spp_aggregation,spp_metrics_services): eliminate N+1 queries and optimize fairness analysis
- Replace per-area child lookup loops with a single OR-chained domain search in aggregation_access.py and service_scope_resolver.py to avoid N+1 queries - Refactor _analyze_many2one_dimension, _analyze_selection_dimension, and _analyze_boolean_dimension in fairness_service.py to use read_group instead of loading entire partner recordsets into memory
1 parent 7123009 commit bb9743c

3 files changed

Lines changed: 88 additions & 42 deletions

File tree

spp_aggregation/models/aggregation_access.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -290,12 +290,15 @@ def _check_explicit_scope_area_compliance(self, partner_ids):
290290

291291
# If include_child_areas is True, expand to include all child areas
292292
if self.include_child_areas:
293-
# Use parent_path for efficient child lookup
294-
for area in self.allowed_area_ids:
295-
if area.parent_path:
296-
# Find all child areas using parent_path prefix
297-
children = self.env["spp.area"].sudo().search([("parent_path", "like", f"{area.parent_path}%")])
298-
allowed_area_ids.update(children.ids)
293+
# Collect all parent_path values first, then do a single search using
294+
# OR-chained domain conditions to avoid N+1 queries inside a loop.
295+
parent_paths = [area.parent_path for area in self.allowed_area_ids if area.parent_path]
296+
if parent_paths:
297+
domain = ["|"] * (len(parent_paths) - 1)
298+
for path in parent_paths:
299+
domain.append(("parent_path", "like", f"{path}%"))
300+
child_areas = self.env["spp.area"].sudo().search(domain)
301+
allowed_area_ids.update(child_areas.ids)
299302

300303
# Get area_ids for the partners
301304
partners = self.env["res.partner"].sudo().browse(partner_ids)

spp_aggregation/models/service_scope_resolver.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,14 +145,17 @@ def _resolve_area_ids(self, area_ids, include_children=True):
145145

146146
# Build area domain (sudo for model reads - callers may be unprivileged)
147147
if include_children:
148-
# Use parent_path for efficient child lookup
148+
# Collect all parent_path values first, then do a single search using
149+
# OR-chained domain conditions to avoid N+1 queries inside a loop.
149150
areas = self.env["spp.area"].sudo().browse(area_ids)
150151
all_area_ids = set(area_ids)
151-
for area in areas:
152-
if area.parent_path:
153-
# Find all child areas using parent_path prefix
154-
children = self.env["spp.area"].sudo().search([("parent_path", "like", f"{area.parent_path}%")])
155-
all_area_ids.update(children.ids)
152+
parent_paths = [area.parent_path for area in areas if area.parent_path]
153+
if parent_paths:
154+
domain = ["|"] * (len(parent_paths) - 1)
155+
for path in parent_paths:
156+
domain.append(("parent_path", "like", f"{path}%"))
157+
child_areas = self.env["spp.area"].sudo().search(domain)
158+
all_area_ids.update(child_areas.ids)
156159
area_ids = list(all_area_ids)
157160

158161
# Find registrants directly in these areas
@@ -241,7 +244,7 @@ def _resolve_spatial_polygon_geometry(self, geojson_str):
241244
return spatial_resolver.resolve_polygon(geojson_str)
242245

243246
# Fallback: no spatial support
244-
_logger.warning("Spatial polygon scope requires spp_aggregation_spatial module. " "Returning empty result.")
247+
_logger.warning("Spatial polygon scope requires spp_aggregation_spatial module. Returning empty result.")
245248
return []
246249

247250
def _resolve_spatial_buffer(self, scope):
@@ -276,7 +279,7 @@ def _resolve_spatial_buffer_params(self, latitude, longitude, radius_km):
276279
return spatial_resolver.resolve_buffer(latitude, longitude, radius_km)
277280

278281
# Fallback: no spatial support
279-
_logger.warning("Spatial buffer scope requires spp_aggregation_spatial module. " "Returning empty result.")
282+
_logger.warning("Spatial buffer scope requires spp_aggregation_spatial module. Returning empty result.")
280283
return []
281284

282285
# -------------------------------------------------------------------------

spp_metrics_services/models/fairness_service.py

Lines changed: 68 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -206,27 +206,39 @@ def _analyze_many2one_dimension(
206206
overall_coverage,
207207
partner_model,
208208
):
209-
"""Analyze a Many2one field dimension."""
209+
"""Analyze a Many2one field dimension.
210+
211+
Uses read_group to count population and beneficiaries per group in two
212+
database queries rather than loading all records into memory.
213+
"""
210214
group_results = []
211215
worst_ratio = 1.0
212216

213-
# Get distinct values from population using mapped() to avoid N+1
214-
population = partner_model.search(base_domain)
215-
values = set(population.mapped(f"{field_name}.id"))
216-
values.discard(False)
217-
218-
for value_id in values:
219-
related_record = self.env[partner_model._fields[field_name].comodel_name].browse(value_id)
220-
221-
group_domain = base_domain + [(field_name, "=", value_id)]
222-
group_total = partner_model.search_count(group_domain)
217+
# Population counts per group - one query
218+
population_groups = partner_model.read_group(base_domain, [field_name], [field_name])
219+
# Beneficiary counts per group - one query
220+
beneficiary_domain = base_domain + [("id", "in", list(beneficiary_set))]
221+
beneficiary_groups = partner_model.read_group(beneficiary_domain, [field_name], [field_name])
222+
223+
# Build lookup dict from group value -> beneficiary count
224+
# read_group returns the Many2one field as (id, display_name) tuple or False
225+
beneficiary_counts = {}
226+
for row in beneficiary_groups:
227+
value = row[field_name]
228+
value_id = value[0] if value else False
229+
beneficiary_counts[value_id] = row[f"{field_name}_count"]
230+
231+
for row in population_groups:
232+
value = row[field_name]
233+
if not value:
234+
continue
235+
value_id, display_name = value
236+
group_total = row[f"{field_name}_count"]
223237

224238
if group_total == 0:
225239
continue
226240

227-
# Count beneficiaries in this group
228-
beneficiary_domain = group_domain + [("id", "in", list(beneficiary_set))]
229-
group_beneficiaries = partner_model.search_count(beneficiary_domain)
241+
group_beneficiaries = beneficiary_counts.get(value_id, 0)
230242
group_coverage = group_beneficiaries / group_total
231243

232244
disparity_ratio = self._compute_disparity_ratio(group_coverage, overall_coverage)
@@ -235,7 +247,7 @@ def _analyze_many2one_dimension(
235247
status = self._get_disparity_status(disparity_ratio)
236248
# For Many2one fields, prefer display_name over dimension label mapping
237249
# since label mappings may use codes rather than database IDs
238-
label = related_record.display_name or dimension.get_label_for_value(str(value_id))
250+
label = display_name or dimension.get_label_for_value(str(value_id))
239251

240252
group_results.append(
241253
{
@@ -269,31 +281,47 @@ def _analyze_selection_dimension(
269281
overall_coverage,
270282
partner_model,
271283
):
272-
"""Analyze a Selection field dimension."""
284+
"""Analyze a Selection field dimension.
285+
286+
Uses read_group to count population and beneficiaries per group in two
287+
database queries rather than one search_count pair per selection value.
288+
"""
273289
group_results = []
274290
worst_ratio = 1.0
275291

276292
field = partner_model._fields[field_name]
277293
selection = field.selection
278294
if callable(selection):
279295
selection = selection(partner_model)
296+
# Build label lookup from the field's selection values
297+
label_by_key = dict(selection)
280298

281-
for key, label in selection:
282-
group_domain = base_domain + [(field_name, "=", key)]
283-
group_total = partner_model.search_count(group_domain)
299+
# Population counts per group - one query
300+
population_groups = partner_model.read_group(base_domain, [field_name], [field_name])
301+
# Beneficiary counts per group - one query
302+
beneficiary_domain = base_domain + [("id", "in", list(beneficiary_set))]
303+
beneficiary_groups = partner_model.read_group(beneficiary_domain, [field_name], [field_name])
304+
305+
# Build lookup dict from selection key -> beneficiary count
306+
beneficiary_counts = {row[field_name]: row[f"{field_name}_count"] for row in beneficiary_groups}
307+
308+
for row in population_groups:
309+
key = row[field_name]
310+
if key is False:
311+
continue
312+
group_total = row[f"{field_name}_count"]
284313

285314
if group_total == 0:
286315
continue
287316

288-
beneficiary_domain = group_domain + [("id", "in", list(beneficiary_set))]
289-
group_beneficiaries = partner_model.search_count(beneficiary_domain)
317+
group_beneficiaries = beneficiary_counts.get(key, 0)
290318
group_coverage = group_beneficiaries / group_total
291319

292320
disparity_ratio = self._compute_disparity_ratio(group_coverage, overall_coverage)
293321
worst_ratio = min(worst_ratio, disparity_ratio)
294322

295323
status = self._get_disparity_status(disparity_ratio)
296-
display_label = dimension.get_label_for_value(key) or label
324+
display_label = dimension.get_label_for_value(key) or label_by_key.get(key, key)
297325

298326
group_results.append(
299327
{
@@ -327,19 +355,31 @@ def _analyze_boolean_dimension(
327355
overall_coverage,
328356
partner_model,
329357
):
330-
"""Analyze a Boolean field dimension."""
358+
"""Analyze a Boolean field dimension.
359+
360+
Uses read_group to count population and beneficiaries per group in two
361+
database queries rather than one search_count pair per boolean value.
362+
"""
331363
group_results = []
332364
worst_ratio = 1.0
333365

334-
for value in [True, False]:
335-
group_domain = base_domain + [(field_name, "=", value)]
336-
group_total = partner_model.search_count(group_domain)
366+
# Population counts per group - one query
367+
population_groups = partner_model.read_group(base_domain, [field_name], [field_name])
368+
# Beneficiary counts per group - one query
369+
beneficiary_domain = base_domain + [("id", "in", list(beneficiary_set))]
370+
beneficiary_groups = partner_model.read_group(beneficiary_domain, [field_name], [field_name])
371+
372+
# Build lookup dict from boolean value -> beneficiary count
373+
beneficiary_counts = {row[field_name]: row[f"{field_name}_count"] for row in beneficiary_groups}
374+
375+
for row in population_groups:
376+
value = row[field_name]
377+
group_total = row[f"{field_name}_count"]
337378

338379
if group_total == 0:
339380
continue
340381

341-
beneficiary_domain = group_domain + [("id", "in", list(beneficiary_set))]
342-
group_beneficiaries = partner_model.search_count(beneficiary_domain)
382+
group_beneficiaries = beneficiary_counts.get(value, 0)
343383
group_coverage = group_beneficiaries / group_total
344384

345385
disparity_ratio = self._compute_disparity_ratio(group_coverage, overall_coverage)

0 commit comments

Comments
 (0)