fix GenericRelation object_id / target id join type mismatch #129
Conversation
django_mongodb/query.py
Outdated
| @@ -115,7 +115,7 @@ def join(self, compiler, connection): | |||
| # The pipeline variables to be matched in the pipeline's | |||
| # expression. | |||
| "let": { | |||
| f"{parent_template}{i}": parent_field | |||
| f"{parent_template}{i}": {"$toString": parent_field} | |||
There was a problem hiding this comment.
I'm not sure that doing toString unconditionally is best. I think Django's SQL joining logic examines the the target field and only casts as necessary. Did you investigate this?
There was a problem hiding this comment.
Not much to be honest, I just asume that the field can be casted as string, but I will do a research of this.
There was a problem hiding this comment.
I found that I have to refactor Cast in order to support it. There is a tiny difference in our flow and postgres (or other sql) flow. But It is doable.
There was a problem hiding this comment.
Never mind, We can't cast to objectId but we can meet in the middle casting to string if the types are different.
WaVEV
force-pushed
the
add-support-for-GenericForeignKey-and-GenericRelation
branch
from
678a08f to
7f53f46
Compare
.github/workflows/test-python.yml
Outdated
| @@ -43,8 +43,8 @@ jobs: | |||
| - name: Checkout Django | |||
| uses: actions/checkout@v4 | |||
| with: | |||
| repository: 'mongodb-forks/django' | |||
| ref: 'mongodb-5.0.x' | |||
| repository: 'wavev/django' | |||
There was a problem hiding this comment.
The test failure is because of some new test on the Django fork. I merged in the changes from your branch, so you can use mongodb-5.0.x again.
Incidentally, can you structure this as two commits:
- "add generic_relations tests to CI"
- the fix (which removes the expected test failures added in commit 1 so it's more clear what's fixed)
WaVEV
force-pushed
the
add-support-for-GenericForeignKey-and-GenericRelation
branch
from
1ca96d9 to
a9e908d
Compare
timgraham
force-pushed
the
add-support-for-GenericForeignKey-and-GenericRelation
branch
from
fd4302d to
8b98b58
Compare
timgraham
changed the title
fixes #121