Skip to content
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

fix: support combination of many-to-one/cacade/explicit composite PK #6417

Merged
merged 1 commit into from
Oct 21, 2020

Conversation

imnotjames
Copy link
Contributor

@imnotjames imnotjames commented Jul 17, 2020

handle setting values deeply in entities with relations in ColumnMetadata,
also check for a virtual relationship column in rawsqlresultstoentitytransformer

this allows us to handle a case where many-to-one with explicit composite PKs
columns were failing to persist a second time- instead of correctly updating the
field they would cause an insert to occur leading to a unique PK constraint error

Fixes #6416
Fixes #4969
Fixes #4122

@imnotjames
Copy link
Contributor Author

imnotjames commented Jul 19, 2020

PS, through doing this I've found that there's a number of ways the surrounding code could be refactored to make something like this easier to update -

  • The status of "virtual relation-created column" - eg, a column that has been created as part of a relation existing without
    an explicit column already being there is mostly implied through a few particular field checks. This should be codified
    either as a getter or as a property that gets set when the ColumnMetadata is created
  • Embedded column code is just peppered throughout everything. It might be worthwhile to have a separate layer - eg, an EmbeddedColumnMetadata extends ColumnMetdata - to support this logic instead of the if blocks.. As it stands, it adds a lot of complexity to the classes.
  • One thing I've found is that there's a number of footguns in the configuration of a schema. It might be worthwile to clamp down on support of features with more explicit error messages being thrown instead of letting things get in a state that's broken. Buuut... that's a bit more vague to implement.
  • Not everything is the same between decorators and the entity schema. You can create some things in decorators that you can't in EntitySchema and some of the namings aren't quite the same. Not sure the best way to fix that, though, without breaking backwards compatibility.. There's also a number of defaults present in the decorators that aren't in EntitySchema - though in some cases it's because the decorators have more knowledge of where they're being associated..

@imnotjames imnotjames changed the title [WIP] Fixing Issue 6416 - Combining ManyToOne, Cascade, & Composite Primary Key causes Unique Constraint issues fix: support combination of many-to-one/cacade/explicit composte PK Jul 19, 2020
@imnotjames imnotjames marked this pull request as ready for review July 19, 2020 18:51
@imnotjames imnotjames changed the title fix: support combination of many-to-one/cacade/explicit composte PK fix: support combination of many-to-one/cacade/explicit composite PK Aug 8, 2020
handle setting values deeply in entities with relations in ColumnMetadata,
also check for a virtual relationship column in rawsqlresultstoentitytransformer

this allows us to handle a case where `many-to-one` with explicit composite PKs
columns were failing to persist a second time- instead of correctly updating the
field they would cause an insert to occur leading to a unique PK constraint error
@imnotjames imnotjames merged commit 9a0497b into typeorm:master Oct 21, 2020
@imnotjames imnotjames deleted the bugfix/issue-6416 branch October 21, 2020 08:17
@imnotjames imnotjames added the hacktoberfest-accepted label hacktoberfest label Oct 23, 2020
zaro pushed a commit to zaro/typeorm that referenced this pull request Jan 12, 2021
)

handle setting values deeply in entities with relations in ColumnMetadata,
also check for a virtual relationship column in rawsqlresultstoentitytransformer

this allows us to handle a case where `many-to-one` with explicit composite PKs
columns were failing to persist a second time- instead of correctly updating the
field they would cause an insert to occur leading to a unique PK constraint error
jankarres pushed a commit to jankarres/typeorm that referenced this pull request Oct 12, 2021
@jmc420
Copy link

jmc420 commented Oct 20, 2023

I just raised an issue which I think is linked to this: #10450

In my case the composite primary key being updated includes the foreign key to the parent and you get a constraint error. When updating a child with a single primary key or a composite key that does not include the foreign key, updating works ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted label hacktoberfest
Projects
None yet
2 participants