Skip to content

Fixed #33414 -- Fixed creating diamond shaped MTI objects for common ancestor with primary key that has a default. #16903

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 15, 2023
Merged

Fixed #33414 -- Fixed creating diamond shaped MTI objects for common ancestor with primary key that has a default. #16903

merged 1 commit into from
Jun 15, 2023

Conversation

Akash-Kumar-Sen
Copy link
Contributor

@Akash-Kumar-Sen Akash-Kumar-Sen commented May 30, 2023 •
edited
Loading

@Akash-Kumar-Sen Akash-Kumar-Sen changed the title Fixed : 33414 Diamond inheritance causes duplicated PK error when creating an object, if the primary key field has a default Fixed #33414 : Diamond inheritance causes duplicated PK error when creating an object, if the primary key field has a default May 31, 2023
@Akash-Kumar-Sen Akash-Kumar-Sen changed the title Fixed #33414 : Diamond inheritance causes duplicated PK error when creating an object, if the primary key field has a default Fixed #33414 -- Diamond inheritance caused duplicated PK error when creating an object, if the primary key field has a default May 31, 2023
LilyFirefly
LilyFirefly approved these changes Jun 4, 2023
View reviewed changes
Copy link
Contributor

@LilyFirefly LilyFirefly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

My only question is does this need more tests?

@Akash-Kumar-Sen
Copy link
Contributor Author

Thank you for reviewing @LilyFoote

My approach while writing tests:

  1. Testcase for the specific case, for which the bug is reported
  2. Check if any previous tests fails.
  3. Try to generate other possible failure scenarios that has the same root cause

The 1 and 2 seems to be done, we may have some scope in the 3rd one.

shangxiao
shangxiao reviewed Jun 5, 2023
View reviewed changes
shangxiao
shangxiao reviewed Jun 5, 2023
View reviewed changes
shangxiao
shangxiao reviewed Jun 5, 2023
View reviewed changes
@Akash-Kumar-Sen Akash-Kumar-Sen requested a review from shangxiao June 5, 2023 12:10
felixxm
felixxm reviewed Jun 6, 2023
View reviewed changes
felixxm
felixxm reviewed Jun 6, 2023
View reviewed changes
@felixxm felixxm self-assigned this Jun 6, 2023
@Akash-Kumar-Sen Akash-Kumar-Sen changed the title Fixed #33414 -- Diamond inheritance caused duplicated PK error when creating an object, if the primary key field has a default Fixed #33414 and #34634 -- Diamond inheritance caused duplicated PK error when creating an object, if the primary key field has a default Jun 6, 2023
@Akash-Kumar-Sen Akash-Kumar-Sen requested a review from felixxm June 7, 2023 00:49
felixxm
felixxm reviewed Jun 7, 2023
View reviewed changes
felixxm
felixxm reviewed Jun 7, 2023
View reviewed changes
@Akash-Kumar-Sen Akash-Kumar-Sen changed the title Fixed #33414 and #34634 -- Diamond inheritance caused duplicated PK error when creating an object, if the primary key field has a default Fixed #33414 -- Diamond inheritance caused duplicated PK error when creating an object, if the primary key field has a default Jun 7, 2023
@Akash-Kumar-Sen Akash-Kumar-Sen requested a review from felixxm June 7, 2023 04:40
@charettes charettes mentioned this pull request Jun 13, 2023
Merged
@felixxm felixxm changed the title Fixed #33414 -- Diamond inheritance caused duplicated PK error when creating an object, if the primary key field has a default Fixed #33414 -- Fixed creating diamond shaped MTI objects for common ancestor with primary key that has a default. Jun 15, 2023
@Akash-Kumar-Sen @charettes @felixxm
Fixed #33414 -- Fixed creating diamond-shaped MTI objects for common …
5d20e02 
…ancestor with primary key that has a default.

Co-authored-by: Simon Charette <[email protected]>
@felixxm felixxm removed the request for review from shangxiao June 15, 2023 06:22
felixxm
felixxm approved these changes Jun 15, 2023
View reviewed changes
Copy link
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Akash-Kumar-Sen Thanks 👍

@felixxm felixxm merged commit 5d20e02 into django:main Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@felixxm felixxm felixxm approved these changes

+3 more reviewers

@charettes charettes charettes left review comments

@shangxiao shangxiao shangxiao left review comments

@LilyFirefly LilyFirefly LilyFirefly approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@felixxm felixxm

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Akash-Kumar-Sen @charettes @LilyFirefly @shangxiao @felixxm