Skip to content

Conversation

LilyFirefly
Copy link
Contributor

Comment on lines +22 to +23
Copy link
Member

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.

Copy link
Contributor Author

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.

@HamaBarhamou
Copy link

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.

@Lotram
Copy link
Contributor

Lotram commented Dec 11, 2023

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 ?

@LilyFirefly
Copy link
Contributor Author

@Lotram My current sticking point is upgrading ForeignKey to support composite keys smoothly. Currently the ForeignObject parent class has some support for composite keys, but it's full of sharp edges. Smoothing those out in ForeignKey isn't easy though - there's a lot of inheritance and composition making it very confusing to know where the right places to change are.

I'm unlikely to make much progress over the winter, but in the spring I'll probably try to take another look.

@Lotram
Copy link
Contributor

Lotram commented Dec 12, 2023

@LilyFoote Ok, I'll try to look into it too, so I can at least understand and review the changes .

@HamaBarhamou
Copy link

@LotramMon point de friction actuel est la mise à niveau ForeignKeypour prendre en charge les clés composites en douceur. Actuellement, la ForeignObjectclasse parent prend en charge les clés composites, mais elle est pleine d'arêtes vives. Cependant , les lisser ForeignKeyn'est pas facile - il y a beaucoup d'héritage et de composition, ce qui rend très déroutant de savoir où se trouvent les bons endroits pour changer.

Il est peu probable que je fasse beaucoup de progrès au cours de l'hiver, mais au printemps, j'essaierai probablement d'y jeter un autre regard.

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

@sarahboyce
Copy link
Contributor

Closing in favour of #18056

@sarahboyce sarahboyce closed this Nov 28, 2024
@LilyFirefly LilyFirefly deleted the composite branch November 28, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants