Repository navigation
perf(geo): optimize the geoFeatures connection (44 → 5 queries) - #471
Merged
nossila merged 1 commit intoOct 8, 2026
Merged
Conversation
geoFeatures used graphene_django's DjangoFilterConnectionField, under which the query_optimizer compiler never runs. A 10-feature page selecting every field cost 44 queries; it now costs 5, and the count no longer grows with the number of rows. - Switch geo_features to query_optimizer.DjangoConnectionField and move filterset_class onto the ObjectType's Meta, where that field reads it from. max_limit=100 is unchanged. - Add GeoJSONFeatureObjectType.pre_optimization_hook: select_related the target document and its content type, prefetch the generic target in one query per content type, and force into only_fields the columns the resolvers read but GraphQL defers. Everything behind `properties` was deferred, because GeoJSONFeatureProperties is a plain graphene type the optimizer cannot map back to model columns, so each field refetched per row. - Resolve properties.target through baseapp_core's request-scoped document cache, seeding the target's mapped_public_id from the DocumentId already loaded. Without it the target's own relay id was one DocumentId query per feature. - Add test_graphql_query_counts.py pinning the count at 5, asserting it stays flat from 10 to 30 features, and covering the shared-target case. test_max_limit_caps_page_size_at_100 now matches query_optimizer's wording; the cap it asserts (100) is unchanged. baseapp/geo/tests: 109 passed. black, isort and flake8 clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015fczpSCpN3kCRs3UBjbity
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
geoFeaturesusedgraphene_django.filter.DjangoFilterConnectionField. Thequery_optimizercompiler never runs under it, sopre_optimization_hookand every fieldoptimizer_hookare dead code — whichcompose-a-block'sconnection-fields.mdnames as the trap that silently discards the whole optimization layer.Measured on a 10-feature page selecting every field: 44 queries, growing linearly with rows. Three separate N+1s, found by reading the captured SQL rather than guessing:
properties.GeoJSONFeaturePropertiesis a plaingraphene.ObjectTyperesolved by attribute access on the feature, so the optimizer cannot tell thatproperties { featureType }reads a model column. It deferred them, and each access fanned out intorefresh_from_db(fields=['feature_type'])— one round trip per field per row.targetwalkstarget_document.content_object: one query for theDocumentId, one for the target.DocumentId.get_public_id_from_objectran once per target, even after the target itself was prefetched.Changes
geo_features→query_optimizer.DjangoConnectionField, withfilterset_classmoved onto the ObjectType'sMetawhere that field reads it from.max_limit=100unchanged.GeoJSONFeatureObjectType.pre_optimization_hook:select_relatedthe target document and its content type,prefetch_relatedthe generic target (one query per content type, not per row), and push intoonly_fieldsthe columns resolvers read but GraphQL defers —geometry,target_document_id, and everypropertiescolumn.resolve_targetgoes throughbaseapp_core's request-scoped document cache and seeds the target'smapped_public_idfrom theDocumentIdalready loaded, which is whatPublicIdResolverreads instead of querying.44 → 5 queries, flat as rows grow.
Testing
baseapp/geo/tests/test_graphql_query_counts.py: pins the count at 5 with a per-query explanation, asserts it does not change from 10 to 30 features, and covers features sharing one target.baseapp/geo/tests/: 109 passed. black, isort, flake8 clean.test_max_limit_caps_page_size_at_100now matchesquery_optimizer's wording (Requesting first 200 records exceeds the limit of 100.). The cap it asserts is unchanged — only the library emitting the message changed.Notes for the reviewer
This activates hooks that were previously inert, so it changes SQL by design. The count test is the guard; per
connection-fields.mda jump in that number means something reverted to the unoptimized field.Not addressed here:
baseapp/files,content_feedandactivity_logare still on the unoptimized connection field. That's pre-existing repo-wide debt, not geo's.🤖 Generated with Claude Code
https://claude.ai/code/session_015fczpSCpN3kCRs3UBjbity