Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes: #17976 DeviceType_Count on Manufacturer #18583

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

renatoalmeidaoliveira
Copy link
Collaborator

Fixes: #17976 DeviceType_Count on Manufacturer

  • Changed get_prefetches_for_serializer to support nested serializers
  • For each nested field select all RelatedObjectCountField fields and create a Prefetch with the correct annotation

@jeremystretch
Copy link
Member

@renatoalmeidaoliveira it would be ideal to solve this by enabling recursion through nested serializers inside get_annotations_for_serializer() if possible. This is because these annotations employ the count_related() utility function, which IIRC works a bit differently from the standard Count(). Did you try going down this path at all?

@renatoalmeidaoliveira
Copy link
Collaborator Author

renatoalmeidaoliveira commented Feb 10, 2025

@jeremystretch that was my first try, i.e. using get_annotations_for_serializer I tryed that in two ways the one proposed by @arthanson in the issue and another version I made.
In both ways the annotated attribute falls inside the base model instead of the nested one and even using the nottation {nested_model}__count_devices.
It's possible to reprodute that behaivour inside nbshell, then I tryed to work on get_prefetches_for_serializer, making it return the fields using Count.
Now I think it's possible to use count_related inside the get_prefetches_for_serializer, gonna try to implement that way.

@renatoalmeidaoliveira
Copy link
Collaborator Author

About using count_related I was able to use it inside the get_prefetches_for_serializer.
Looking at the query at Django Debug we have the following differences:

Using count_related

SELECT "dcim_manufacturer"."id",
       "dcim_manufacturer"."created",
       "dcim_manufacturer"."last_updated",
       "dcim_manufacturer"."custom_field_data",
       "dcim_manufacturer"."name",
       "dcim_manufacturer"."slug",
       "dcim_manufacturer"."description",
       COALESCE((SELECT COUNT(*) AS "c" FROM "dcim_manufacturer" U0 INNER JOIN "dcim_devicetype" U1 ON (U0."id" = U1."manufacturer_id") WHERE U1."id" = ("dcim_manufacturer"."id") GROUP BY U1."id"), 0) AS "devicetype_count"
  FROM "dcim_manufacturer"
 WHERE "dcim_manufacturer"."id" IN (3)

Performance using count_related

count_related

Using Count

SELECT "dcim_manufacturer"."id",
       "dcim_manufacturer"."created",
       "dcim_manufacturer"."last_updated",
       "dcim_manufacturer"."custom_field_data",
       "dcim_manufacturer"."name",
       "dcim_manufacturer"."slug",
       "dcim_manufacturer"."description",
       COUNT("dcim_devicetype"."id") AS "devicetype_count"
  FROM "dcim_manufacturer"
  LEFT OUTER JOIN "dcim_devicetype"
    ON ("dcim_manufacturer"."id" = "dcim_devicetype"."manufacturer_id")
 WHERE "dcim_manufacturer"."id" IN (3)
 GROUP BY "dcim_manufacturer"."id"

Performance using Count

Count

Summary

In my environment, using NetBox demo data, the performance using Count was a bit better, but both ways works.
Please let me know the preffered way.

@renatoalmeidaoliveira
Copy link
Collaborator Author

About changing get_annotations_for_serializer, I tryed with this approach:

def get_annotations_for_serializer(serializer_class, fields_to_include=None):
   """
   Return a mapping of field names to annotations to be applied to the queryset for a serializer.
   """
   annotations = {}
   # If specific fields are not specified, default to all
   if not fields_to_include:
       fields_to_include = serializer_class.Meta.fields

   model = serializer_class.Meta.model

   for field_name, field in serializer_class._declared_fields.items():
       if issubclass(type(field), Serializer):
           nested_fields = field.Meta.brief_fields if field.nested else []
           for subfield_name, subfield in field.get_fields().items():
               if subfield_name in nested_fields and type(subfield) is RelatedObjectCountField:
                   subfield_model = field.Meta.model
                   annotations[f'{field_name}__{subfield_name}'] = count_related(subfield_model, subfield.relation)
       if field_name in fields_to_include and type(field) is RelatedObjectCountField:
           related_field = getattr(model, field.relation).field
           annotations[field_name] = count_related(related_field.model, related_field.name)
   return annotations

Running with this with Debug it executes the following query:

SELECT "dcim_devicetype"."id",
       "dcim_devicetype"."created",
       "dcim_devicetype"."last_updated",
       "dcim_devicetype"."custom_field_data",
       "dcim_devicetype"."description",
       "dcim_devicetype"."comments",
       "dcim_devicetype"."weight",
       "dcim_devicetype"."weight_unit",
       "dcim_devicetype"."_abs_weight",
       "dcim_devicetype"."manufacturer_id",
       "dcim_devicetype"."model",
       "dcim_devicetype"."slug",
       "dcim_devicetype"."default_platform_id",
       "dcim_devicetype"."part_number",
       "dcim_devicetype"."u_height",
       "dcim_devicetype"."exclude_from_utilization",
       "dcim_devicetype"."is_full_depth",
       "dcim_devicetype"."subdevice_role",
       "dcim_devicetype"."airflow",
       "dcim_devicetype"."front_image",
       "dcim_devicetype"."rear_image",
       "dcim_devicetype"."console_port_template_count",
       "dcim_devicetype"."console_server_port_template_count",
       "dcim_devicetype"."power_port_template_count",
       "dcim_devicetype"."power_outlet_template_count",
       "dcim_devicetype"."interface_template_count",
       "dcim_devicetype"."front_port_template_count",
       "dcim_devicetype"."rear_port_template_count",
       "dcim_devicetype"."device_bay_template_count",
       "dcim_devicetype"."module_bay_template_count",
       "dcim_devicetype"."inventory_item_template_count",
       COALESCE((SELECT COUNT(*) AS "c" FROM "dcim_manufacturer" U0 INNER JOIN "dcim_devicetype" U1 ON (U0."id" = U1."manufacturer_id") WHERE U1."id" = ("dcim_devicetype"."id") GROUP BY U1."id"), 0) AS "manufacturer__devicetype_count",
       COALESCE((SELECT COUNT(*) AS "c" FROM "dcim_platform" U0 INNER JOIN "dcim_device" U1 ON (U0."id" = U1."platform_id") WHERE U1."id" = ("dcim_devicetype"."id") GROUP BY U1."id"), 0) AS "default_platform__device_count",
       COALESCE((SELECT COUNT(*) AS "c" FROM "dcim_platform" U0 INNER JOIN "virtualization_virtualmachine" U1 ON (U0."id" = U1."platform_id") WHERE U1."id" = ("dcim_devicetype"."id") GROUP BY U1."id"), 0) AS "default_platform__virtualmachine_count",
       COALESCE((SELECT COUNT(*) AS "c" FROM "dcim_device" U0 WHERE U0."device_type_id" = ("dcim_devicetype"."id") GROUP BY U0."device_type_id"), 0) AS "device_count"
  FROM "dcim_devicetype"
 WHERE "dcim_devicetype"."id" = 7
 LIMIT 21

And looking at the results inside Django debug, its returning the correct results but as an attribute of DeviceType :

image

So even the query returning the expected results the JSON output omits the result instead of adding it inside the Nested Object

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DeviceType_Count on Manufacturer
2 participants