-
-
Notifications
You must be signed in to change notification settings - Fork 438
Added m2m related table alias #1972
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
base: develop
Are you sure you want to change the base?
Conversation
@sietr0s thank you for the contribution! Can you please add a test exposing the issue? This requirement was a part of the PR template :) Update: there are failing tests. |
CodSpeed Performance ReportMerging #1972 will not alter performanceComparing Summary
|
Fix problems alias
Pull Request Test Coverage Report for Build 15905581699Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a test exposing the original issue? Meaning that the new test should fail without your changes. This is a requirement for a PR to be merged. Thanks!
f"{table.get_table_name()}__{iter_field.model_field_name}" | ||
) | ||
if isinstance(related_field, ManyToManyFieldInstance): | ||
related_table = related_table.as_( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code duplicate the code of the check above. You can rewrite it as
if isinstance(related_field, (ForeignKeyFieldInstance, ManyToManyFieldInstance)):
related_table = related_table.as_( | ||
f"{table.get_table_name()}__{related_field.model_field_name}" | ||
) | ||
if isinstance(related_field, ManyToManyFieldInstance): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
elif
should prevent a redundant check.
Fix incorrect JOIN aliasing when filtering on multiple ManyToMany relations to the same table
Description
This PR fixes an issue where Tortoise ORM fails to properly alias JOINs when filtering on two different ManyToMany relations pointing to the same model. The problem occurred because
get_joins_for_related_field
function didn't apply table aliasing for ManyToManyFieldInstance cases.The main change modifies the
get_joins_for_related_field
function to add proper table aliasing for ManyToMany relations by using the pattern{table_name}__{field_name}
for the alias.Motivation and Context
When making queries that filter on two different ManyToMany relations to the same model (through different through tables), Tortoise ORM was generating SQL without proper table aliasing, causing ambiguity in the JOINs. This made such queries impossible to execute correctly.
Fixes issue #1969