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

Breaking change for many-to-many relationships in 0.2.33 #7745

Closed
2 of 21 tasks
nbryan opened this issue Jun 15, 2021 · 6 comments
Closed
2 of 21 tasks

Breaking change for many-to-many relationships in 0.2.33 #7745

nbryan opened this issue Jun 15, 2021 · 6 comments

Comments

@nbryan
Copy link

nbryan commented Jun 15, 2021

Issue Description

When upgrading from an older version of TypeORM I noticed my schema was no longer in sync. I isolated the breaking change to version 0.2.33 (0.2.34 is current).

Previously, the FKs on the join table were all set to ON DELETE CASCADE by default (and ON UPDATE NO ACTION).

Expected Behavior

When I run typeorm schema:log I should see:

Your schema is up to date - there are no queries to be executed by schema syncronization.

Actual Behavior

After upgrading typeorm schema:log outputs:

----------------------------------------------------------------
-- Schema syncronization will execute following sql queries (4):
----------------------------------------------------------------
ALTER TABLE "pa_child_invoice" DROP CONSTRAINT "FK_75b46678a96525a992cf4f05b9f";
ALTER TABLE "pa_child_invoice" DROP CONSTRAINT "FK_f0b0fc53801ca38819ecc925df4";
ALTER TABLE "pa_child_invoice" ADD CONSTRAINT "FK_75b46678a96525a992cf4f05b9f" FOREIGN KEY ("paInvoiceId") REFERENCES "pa_invoice"("id") ON DELETE CASCADE ON UPDATE CASCADE;
ALTER TABLE "pa_child_invoice" ADD CONSTRAINT "FK_f0b0fc53801ca38819ecc925df4" FOREIGN KEY ("childInvoiceId") REFERENCES "pa_invoice"("id") ON DELETE NO ACTION ON UPDATE NO ACTION;

Steps to Reproduce

  1. Create an entity with a many-to-many relationship with itself using TypeORM <= 0.2.32.
  2. Sync the database schema.
  3. Upgrade to TypeORM >= 0.2.33.
  4. Run typeorm schema:log.

Here's the relevant snippet from my entity:

    @ManyToMany(() => Invoice, (i) => i.parentInvoices, { cascade: true })
    @JoinTable({
        name: 'pa_child_invoice',
        joinColumn: { name: 'paInvoiceId' },
        inverseJoinColumn: { name: 'childInvoiceId' }
    })
    childInvoices: Invoice[];

    @ManyToMany(() => Invoice, (i) => i.childInvoices)
    parentInvoices: Invoice[];

I'm not sure if it's important that my entity be self-referential or not. In my case an Invoice may have many child invoices, and an invoice may also have many parent invoices.

My Environment

Dependency Version
Operating System macOS 11.3.1 Big Sur
Node.js version v14.17.0
Typescript version v4.2.4
TypeORM version v0.2.33

Additional Context

I was able to work around the issue by changing my entity to explicitly set the cascading behavior of the foreign keys:

    @ManyToMany(() => Invoice, (i) => i.parentInvoices, { cascade: true, onDelete: 'CASCADE', onUpdate: 'NO ACTION' })
    @JoinTable({
        name: 'pa_child_invoice',
        joinColumn: { name: 'paInvoiceId' },
        inverseJoinColumn: { name: 'childInvoiceId' }
    })
    childInvoices: Invoice[];

    @ManyToMany(() => Invoice, (i) => i.childInvoices, { onDelete: 'CASCADE' })
    parentInvoices: Invoice[];

Relevant Database Driver(s)

  • aurora-data-api
  • aurora-data-api-pg
  • better-sqlite3
  • cockroachdb
  • cordova
  • expo
  • mongodb
  • mysql
  • nativescript
  • oracle
  • postgres
  • react-native
  • sap
  • sqlite
  • sqlite-abstract
  • sqljs
  • sqlserver

Are you willing to resolve this issue by submitting a Pull Request?

  • Yes, I have the time, and I know how to start.
  • Yes, I have the time, but I don't know how to start. I would need guidance.
  • No, I don't have the time, although I believe I could do it if I had the time...
  • No, I don't have the time and I wouldn't even know how to start.

Since I have a workaround this isn't too pressing for me, so I don't intend to investigate further or open a pull request. Though if I knew more about TypeORM internals I probably would. It also occurred to me that, since this isn't the expected behavior with other foreign keys, just for join tables, this might be the expected behavior anyway and that the previous behavior was incorrect.

@pwillcode
Copy link

pwillcode commented Jun 17, 2021

I am also having trouble with this version number, although my problem is that TypeORM doesn't see any changes when it should. My only worksround currently is to use TypeORM 0.2.32.

I don't have time to get into details right now, but could try to provide more if needed.
I am also on Mac, using node 16.2.0 and typescript 4.2.4, and the Postgres driver.

@imnotjames
Copy link
Contributor

I was able to work around the issue by changing my entity to explicitly set the cascading behavior of the foreign keys:

Isn't this more correct?

Shouldn't cascading deletes be EXPLICITLY defined because of how destructive / dangerous they are?

@nbryan
Copy link
Author

nbryan commented Jun 18, 2021

@imnotjames It might be, because you're right, all other FK relationships require one to explicitly set the cascading behavior. But a couple things:

  1. This previously worked one way, and now it doesn't, so it is a breaking change even if it is now more correct. Given it didn't appear in the changelog it also seemed to be an unintentional change, which is worrying.
  2. On the owning side of the relationship I also had to set onUpate: 'NO ACTION' which should have remained the default, and indeed still is the default on all other FK relationships.

Part of the reason for opening this issue was to get clarification on this. I'm fine if the answer to point 1 is "won't fix, as designed" or whatever, but point 2 does seem like a bug.

@imnotjames
Copy link
Contributor

It's not clear in the documentation but default is supposed to not cascade from my reading.

What were the constraints set to before?

@imnotjames imnotjames self-assigned this Jun 18, 2021
@nbryan
Copy link
Author

nbryan commented Jun 18, 2021

Before the FKs were set to ON DELETE CASCADE and ON UPDATE NO ACTION. After this change the the owning side of the join table was set to ON DELETE CASCADE ON UPDATE CASCADE and the other side set to ON DELETE NO ACTION ON UPDATE NO ACTION

So it seems the cascading delete on the owning side didn't change, just the cascading update. On the other side the cascading delete changed, and the the cascading update did not.

@imnotjames
Copy link
Contributor

imnotjames commented Jun 19, 2021

Seems related #5714

With that in mind this makes sense as the expected behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants