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

DML statement cannot have any enabled triggers if the statement contains an OUTPUT clause without INTO clause #5160

Closed
abobija opened this issue Nov 30, 2019 · 7 comments · Fixed by #5361

Comments

@abobija
Copy link

abobija commented Nov 30, 2019

Issue type:

[ ] question
[x] bug report
[ ] feature request
[ ] documentation issue

Database system/driver:

[ ] cordova
[ ] mongodb
[x] mssql
[ ] mysql / mariadb
[ ] oracle
[ ] postgres
[ ] cockroachdb
[ ] sqlite
[ ] sqljs
[ ] react-native
[ ] expo

TypeORM version:

[x] latest
[ ] @next
[ ] 0.x.x (or put your version here)

Steps to reproduce or a small repository showing the problem:

I have tried to insert Person into MSSQL table which contains one trigger, and I have run into this exception:

The target table \'Person.person\' of the DML statement cannot have any enabled triggers if the statement contains an OUTPUT clause without INTO clause.

Typeorm tried to execute this query:

exec sp_executesql @statement=N'INSERT INTO "Person"."person"("FirstName", "LastName", "Title") OUTPUT INSERTED."BusinessEntityID" VALUES (@0, @1, @2)',@params=N'@0 nvarchar(50), @1 nvarchar(50), @2 nvarchar(8)',@0=N'Alija',@1=N'Bobija',@2=N'Mr.'

I have used database AdventureWorks2017 that is open source and available on Microsoft online websites.

This is my Person entity:

@Entity({
    schema: 'Person'
})
export default class Person extends BaseEntity {
    @PrimaryGeneratedColumn({
        name: 'BusinessEntityID'
    })
    Id!: number;

    @Column({
        length: 50
    })
    FirstName! : string;

    @Column({
        length: 50
    })
    LastName!: string;

    @Column({
        length: 8
    })
    Title!: string;
}

And this is my code for adding new person:

    await Person.insert({
        FirstName: 'Alija',
        LastName: 'Bobija',
        Title: 'Mr.'
    });

Best regards

@abobija abobija closed this as completed Nov 30, 2019
@abobija abobija reopened this Nov 30, 2019
@sgarner
Copy link
Collaborator

sgarner commented Dec 2, 2019

See #2311

There is no real fix for this currently. You can set the connection option options { disableOutputReturning: true } to disable OUTPUT clauses, but then TypeORM will not update entity instances with generated values from inserts or updates correctly.

I think the answer is the SqlServer driver needs to be modified so it uses OUTPUT...INTO instead.

@abobija
Copy link
Author

abobija commented Dec 2, 2019

Hey, thank you for your reply.

I have read #2311 and try with disableOutputReturning and it works, but I really need that TypeORM power of updating entity instances with generated values from inserts and updates.

Is there any possibility to upgrade SqlServer driver to use OUTPUT...INTO clauses? Because I'm using triggers in my project very frequently.

I'm willing to work on this feature, but I need just few tips from where I need to start.

@codymartin
Copy link

This is a problem for me to. I'm inserting into a table with a trigger and i need the identity back on the insert. Any update on updating the driver?

@abobija
Copy link
Author

abobija commented Jan 9, 2020

C'mon guyz, we need this, let's do it 😃

@sgarner
Copy link
Collaborator

sgarner commented Jan 13, 2020

@pleerock Do you have any guidance on this, are you happy for someone to make a PR?

This issue is quite serious. 🙁 I can't get rid of the triggers on my table so I had to set disableOutputReturning. But now manager.create() returns back the first row from the table instead of the newly created entity, resulting in bugs that are a pain to work around.

@sgarner
Copy link
Collaborator

sgarner commented Jan 15, 2020

I made a PR: #5361

pleerock pushed a commit that referenced this issue Jan 21, 2020
* test: Add test for GitHub issue #5160

* fix: use OUTPUT INTO on SqlServer for returning columns

Closes #5160

* chore: fix typo in comment

* fix: use full types with length e.g. nvarchar(255)

* refactor: move output table declaration to driver

* fix: don't try to drop temporary tables
@abobija
Copy link
Author

abobija commented Jan 25, 2020

Hi, sorry for late response.
@sgarner thank you very very much for this contribution!
Now I can finally continue with my project, woohoo!
👍

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 a pull request may close this issue.

3 participants