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. #27

Open
LoganBresnahan opened this issue Sep 20, 2017 · 10 comments
Labels

Comments

@LoganBresnahan
Copy link

Problem:

When a database table has a trigger attached to it, insert/1 will fail because the adapter is not utilizing an INTO clause.

This blog post from the SQL Server team explains the issue well: https://blogs.msdn.microsoft.com/sqlprogrammability/2008/07/11/update-with-output-clause-triggers-and-sqlmoreresults/

Error Example:

m=%{
  client_sport_id: 1, 
  user_id_entered: 1, 
  tracking_type: "TT-mon", 
  subject: "Incoming Call", 
  notes: "This is a note.", 
  tracking_date: "2017-09-14T11:21:39.592873Z",
  entry_date: "2017-09-14T11:21:39.592873Z"
}

cs=OpsMessagingService.ClientTracking.changeset(%OpsMessagingService.ClientTracking{},m)

OpsMessagingService.Repo.insert(cs)

result:

insert/1 error:
** (Mssqlex.Error) [Microsoft][ODBC Driver 13 for SQL Server][SQL Server]The target table 'client_tracking' of the DML statement cannot have any enabled triggers if the statement contains an OUTPUT clause without INTO clause. | ODBC_CODE 42000 | SQL_SERVER_CODE 334
    (ecto) lib/ecto/adapters/sql.ex:571: Ecto.Adapters.SQL.struct/7
    (ecto) lib/ecto/repo/schema.ex:469: Ecto.Repo.Schema.apply/4
    (ecto) lib/ecto/repo/schema.ex:205: anonymous fn/13 in Ecto.Repo.Schema.do_insert/4

DB Trigger Example:

CREATE TRIGGER [dbo].[briu_client_tracking] on [dbo].[client_tracking]
  for INSERT,UPDATE
  as
begin
  declare  
           @icnt int,
           @dcnt int,
           @errno   int,
           @errmsg  varchar(255)
           
   begin
     select @icnt = count(*) from inserted;
     select @dcnt = count(*) from deleted;
     IF @icnt > 0 and @dcnt=0
       begin
         update client_tracking
         set entry_date=current_timestamp,
          mod_date=current_timestamp
          where client_tracking_id in (select client_tracking_id from inserted)
	 
       end
     IF @dcnt > 0 
        begin 
          update client_tracking
          set mod_date=current_timestamp
                where client_tracking_id in (select client_tracking_id from deleted)

        end
    end
return      

end

Temporary Workaround:

Even if you're only looking to insert one record, you can use insert_all/2 and pass in your one entry because it does not rely on a return value from the database.
https://hexdocs.pm/ecto/Ecto.Repo.html#c:insert_all/3

Environment:

  • MssqlEcto version: 0.1.4
  • Ecto version: 2.1.6
  • Elixir version: 1.4
  • Erlang/OTP version: 19
  • Microsoft SQL Server version: Microsoft SQL Server 2014 (SP2-CU5) - 12.0.5546.0 (X64)
  • Operating System and version: Ubuntu 16.04.3 LTS
@jbachhardie jbachhardie added the bug label May 7, 2018
@iDarshn
Copy link

iDarshn commented Jun 12, 2018

I am facing the same issue. Do you guys have any update on it?

@harri88
Copy link

harri88 commented Jul 10, 2018

any update for this issue?

@jbachhardie
Copy link

I've just come back from a month's holiday and things are a bit less hectic now than they were for the first half of the year so I'm going to try to dedicate at least a few hours a week to updating this library. I'll prioritise merging PRs, since my personal Elixir skills are rather rusty at this point, but if nobody else is picking this issue up I'll get to it.

@cpursley
Copy link

cpursley commented Jul 13, 2018

I just ran into this issue as well. I'd be glad to try my hand at patching this if someone could provide some pointers on where to start in this codebase.

This seems related:

@cpursley
Copy link

cpursley commented Jul 13, 2018

It looks like making inserted an optional parameter is one possible solution: https://github.com/findmypast-oss/mssql_ecto/blob/master/lib/mssql_ecto/query.ex#L107

Unfortunately, we won't be able to get the inserted data back with this approach.

@cpursley
Copy link

cpursley commented Jul 16, 2018

Another alternative might be to check if a trigger exists on a table and dynamically add/remove the inserted bit: https://wateroxconsulting.com/archives/find-triggers-sql-server-database/

But perhaps this would add too much overhead?

@jbachhardie
Copy link

Does setting returning: false on the insert/2 call allow the query to complete? I think that might be the best we can do, allow the user to turn off OUTPUT. I don't want to have every DML statement have to make an extra query to figure out if there are triggers and I don't think we can get the ODBC adapter we depend on to do the multiline statement necessary to output into a temp table and then select from it (though I might be wrong there).

@cpursley
Copy link

cpursley commented Jul 17, 2018

I think you're right (allow the user to turn off OUTPUT).

Setting returning: false didn't work for me. Where is insert/2 defined? All I'm seeing is this def: https://github.com/findmypast-oss/mssql_ecto/blob/master/lib/mssql_ecto/query.ex#L98

@jbachhardie
Copy link

I meant Ecto.Repo.insert/2 which is how the end-user interacts with the adapter. I also think I got it wrong and "return nothing" is expressed as returning: [] rather than false, which I believe gets translated into the returning argument being [] in query.ex which looking at the code should remove the OUTPUT clause.

@apurvaraut
Copy link

In the db you can delete the trigger which is not in use. can solve this issue. I did delete the trigger from the table . and it worked fine.

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

No branches or pull requests

6 participants