-
-
Notifications
You must be signed in to change notification settings - Fork 33.1k
Fixed #373 -- Support multiple column fields #17279
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
Conversation
67dcefa
to
8ac4425
Compare
django/db/models/fields/composite.py
Outdated
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.
👋 @LilyFoote! Just passing by as there was some recent interest in this area from the forum to mention that if you're looking to implement TupleIn
there is some pre-existing logic to support it for joins at least.
See ticket-19385, and the ForeignObject
, MultiColSource
, SQLQuery.build_filter
, RelatedIn
symbols for reference.
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.
Thanks for the pointers! Where I'm currently stuck is working out how to adapt ForeignKey
to point to a CompositeField
. There's something about how ForeignKey
maps to the database column(s) that's just not clicked yet.
I am delighted to see the significant progress on ticket 373. As this ticket led to the closure of ticket 5929, which I was actively following and working on, I am particularly interested in the development and implementation of this feature. While my work on ticket 5929 is still in local development and not submitted via a pull request, I will be keenly observing the advancements of ticket 373, eager to see how these developments might influence and enhance my own contributions. Thank you for your ongoing commitment to improving Django. |
Hey @LilyFoote , I'd really like to see this PR going forward. Is there any way I can help ? Do you know what's is still needed for this to be merged ? |
@Lotram My current sticking point is upgrading I'm unlikely to make much progress over the winter, but in the spring I'll probably try to take another look. |
@LilyFoote Ok, I'll try to look into it too, so I can at least understand and review the changes . |
Hi @LilyFoote , Thank you for sharing your current challenge with upgrading ForeignKey to support composite keys. It sounds like a complex task, especially with the intricacies of inheritance and composition in the codebase. To assist you more effectively and refine our approach, it would be incredibly helpful if you could share any specific error codes, examples, or detailed descriptions of the issues you're encountering. Understanding the precise nature of these 'sharp edges' you mentioned will enable us to offer more targeted insights and suggestions. If possible, any code snippets or scenarios where the current implementation falls short would be greatly valuable. This information will not only help in reproducing the issues but also in brainstorming potential solutions or workarounds. Looking forward to diving deeper into this with you in the spring, and hoping to contribute to smoothing out these challenges in the ForeignKey upgrade. Best regard |
Co-authored-by: Lily Foote <[email protected]>
We can't depend on `get_joining_fields` here because that breaks `foreign_object.tests.GetJoiningDeprecationTests.test_join_get_joining_columns_warning`.
This perhaps should raise `TypeError`, but the equivalent case with a non-composite primary key doesn't raise either.
This wasn't really adding anything except maybe implicitly testing `Meta.ordering`.
Closing in favour of #18056 |
https://code.djangoproject.com/ticket/373
Continued from #16075.