Skip to content

Commit b6d283b

Browse files
committed
fix(spp_gis_report): fix aggregation logic for sum/min/max and N+1 query
- For sum: accumulate the subgroup sum directly instead of multiplying by count (which inflated results) - For min/max: track the true minimum/maximum across subgroups rather than multiplying by count (which was meaningless) - For avg: behaviour unchanged (weighted accumulation then divide) - Replace per-base-area descendant search loop with a single batched query followed by in-memory parent-chain traversal, eliminating the N+1 query pattern
1 parent fa5ae93 commit b6d283b

File tree

1 file changed

+39
-15
lines changed

1 file changed

+39
-15
lines changed

spp_gis_report/models/gis_report.py

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -472,17 +472,23 @@ def _compute_base_aggregation(self):
472472
# (e.g., barangays when base level is municipality). We include all
473473
# descendant areas in the query and aggregate results back to the
474474
# base-level parent.
475-
child_to_base = {}
476-
for base_area in base_areas:
477-
child_to_base[base_area.id] = base_area.id
478-
descendants = self.env["spp.area"].search(
479-
[
480-
("id", "child_of", base_area.id),
481-
("id", "!=", base_area.id),
482-
]
483-
)
484-
for desc in descendants:
485-
child_to_base[desc.id] = base_area.id
475+
#
476+
# Fetch all descendants of all base areas in a single query, then build
477+
# the child_to_base mapping in memory to avoid an N+1 query pattern.
478+
child_to_base = {base_area.id: base_area.id for base_area in base_areas}
479+
all_descendants = self.env["spp.area"].search(
480+
[
481+
("id", "child_of", base_areas.ids),
482+
("id", "not in", base_areas.ids),
483+
]
484+
)
485+
# For each descendant, walk up its parent chain to find the base-level ancestor
486+
for desc in all_descendants:
487+
ancestor = desc.parent_id
488+
while ancestor and ancestor.id not in child_to_base:
489+
ancestor = ancestor.parent_id
490+
if ancestor:
491+
child_to_base[desc.id] = child_to_base[ancestor.id]
486492

487493
# Add area filter to domain (base areas + all descendants)
488494
domain.append((area_field, "in", list(child_to_base.keys())))
@@ -525,12 +531,30 @@ def _compute_base_aggregation(self):
525531
base_id = child_to_base.get(area_id, area_id)
526532
value = group.get(self.aggregation_field) or 0
527533
count = group[f"{area_field}_count"]
528-
# Accumulate weighted values for proper re-aggregation
529-
results[base_id]["raw"] += value * count
534+
if agg_func == "avg":
535+
# Accumulate weighted sum so we can compute a proper
536+
# weighted average across subgroups when rolling up.
537+
results[base_id]["raw"] += value * count
538+
elif agg_func == "sum":
539+
# read_group already returns the sum for the subgroup;
540+
# multiplying by count would inflate the total.
541+
results[base_id]["raw"] += value
542+
elif agg_func == "min":
543+
# Keep the lowest value seen across subgroups.
544+
if results[base_id]["count"] == 0:
545+
results[base_id]["raw"] = value
546+
else:
547+
results[base_id]["raw"] = min(results[base_id]["raw"], value)
548+
elif agg_func == "max":
549+
# Keep the highest value seen across subgroups.
550+
if results[base_id]["count"] == 0:
551+
results[base_id]["raw"] = value
552+
else:
553+
results[base_id]["raw"] = max(results[base_id]["raw"], value)
530554
results[base_id]["count"] += count
531555
results[base_id]["weight"] += count
532556

533-
# For avg: convert accumulated (value*count) back to weighted average
557+
# For avg: convert accumulated weighted sum back to weighted average
534558
if agg_func == "avg":
535559
for data in results.values():
536560
if data["count"] > 0:
@@ -1514,7 +1538,7 @@ def _to_geojson(
15141538
feature["geometry"] = shape.__geo_interface__
15151539
except ImportError:
15161540
_logger.warning(
1517-
"shapely not available, geometry export limited. " "Install shapely for full geometry support."
1541+
"shapely not available, geometry export limited. Install shapely for full geometry support."
15181542
)
15191543
feature["geometry"] = None
15201544
except Exception as e:

0 commit comments

Comments
 (0)