This repository was archived by the owner on Sep 3, 2025. It is now read-only.
fix(db): fixes joined model comparison error#6011
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes issues with tracking joined models in query construction by unifying model identity and preventing redundant joins.
- Tracks the actual model/table via
parentor itself to ensure consistent identity. - Checks and skips already-joined entities to avoid duplicate joins.
Comments suppressed due to low confidence (2)
src/dispatch/database/service.py:489
- There’s no test coverage for the new deduplication logic. Add tests that verify both standard and alias-based joins are correctly added or skipped.
query = query.join(joined_model, isouter=is_outer)
src/dispatch/database/service.py:487
- [nitpick] The variable name
model_or_tableis generic. Consider renaming tojoin_targetorentity_keyto more clearly express its role in deduplication.
model_or_table = getattr(joined_model, "parent", joined_model)
ghost
approved these changes
May 22, 2025
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
In our current implementation, we encountered an error when attempting to join models within our query construction logic. The error message indicated:
This issue arose because our logic was attempting to track joined models using
InstrumentedAttributeobjects, which led to incorrect comparisons based on object identity rather than logical identity.Changes Made:
Tracking Models Directly:
InstrumentedAttributeobjects. This is done by using theparentattribute when available, and defaulting to thejoined_modelitself whenparentis not present. This ensures that we are consistently tracking the logical entities being joined, regardless of whether they are accessed via ORM attributes or directly as tables.Avoiding Duplicate Joins:
joined_models) and checking for membership before appending new joins. This prevents redundant joins and helps maintain query efficiency.