tags:

views:

3256

answers:

5

Here's something I haven't been able to fix, and I've looked everywhere. Perhaps someone here will know!

I have a table called dandb_raw, with three columns in particular: dunsId (PK), name, and searchName. I also have a trigger that acts on this table:

SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO

ALTER TRIGGER [dandb_raw_searchNames]
    ON [dandb_raw]
    FOR INSERT, UPDATE
    AS

SET NOCOUNT ON

  select dunsId, name into #magic from inserted

  UPDATE dandb
   SET dandb.searchName = company_generateSearchName(dandb.name)
   FROM (select dunsId, name from #magic) i
   INNER JOIN dandb_raw dandb
    on i.dunsId = dandb.dunsId


  --Add new search matches
  SELECT c.companyId, dandb.dunsId
   INTO #newMatches
   FROM dandb_raw dandb
   INNER JOIN (select dunsId, name from #magic) a
    on a.dunsId = dandb.dunsId
   INNER JOIN companies c
    ON dandb.searchName = c.searchBrand
    --avoid url matches that are potentially wrong
    AND (lower(dandb.url) = lower(c.url)
     OR dandb.url = ''
     OR c.url = ''
     OR c.url is null)


  INSERT INTO #newMatches (companyId, dunsId)
  SELECT c.companyId, max(dandb.dunsId) dunsId
   FROM dandb_raw dandb
   INNER JOIN
    (
     select
     case when charindex('/',url) <> 0 then left(url, charindex('/',url)-1)
     else url
     end urlMatch, * from companies
    ) c
    ON dandb.url = c.urlMatch
   where subsidiaryOf = 1 and isReported = 1 and dandb.url <> ''
    and c.companyId not in (select companyId from #newMatches)
   group by companyId
   having count(dandb.dunsId) = 1

  UPDATE cd
   SET cd.dunsId = nm.dunsId
   FROM companies_dandb cd
   INNER JOIN #newMatches nm
    ON cd.companyId = nm.companyId

GO

The trigger causes inserts to fail:

insert into  [dandb_raw](dunsId, name)
 select 3442355, 'harper'
 union all
 select 34425355, 'har 466per'
update [dandb_raw] set name ='grap6767e'

With this error:

Msg 213, Level 16, State 1, Procedure companies_contactInfo_updateTerritories, Line 20
Insert Error: Column name or number of supplied values does not match table definition.

The most curious thing about this is that each of the individual statements in the trigger works on its own. It's almost as though inserted is a one-off table that infects temporary tables if you try to move inserted into one of them.

So what causes the trigger to fail? How can it be stopped?

+1  A: 

What is companies_contactInfo_updateTerritories? The actual reference mentions procedure "companies_contactInfo_updateTerritories" but I do not see it in the code given. Also I do not see where it is being called. Unless it is from your application that is calling the SQL and hence irrelevant....

If you tested everything and it worked but now it doesn't work, then something must be different. One thing to consider is security. I noticed that you just call the table [dandb_raw] and not [dbo].[dandb_raw]. So if the user had a table of the same name [user].[dandb_raw], that table would be used to check the definitions instead of your table. Also, the trigger creates temp tables. But if some of the temp tables already existed for whatever reason but with different definitions, this may also be a problem.

Cervo
That was very useful...I had missed the trigger companies_contactInfo_updateTerritories. I'm going to look more into it and see if I can pin down the problem.
Chris
+1  A: 

I don't see any obvious problem in the code.

"SELECT .. INTO" is weak kung-fu. Try explicitly creating the temp table definition:

CREATE TABLE #newMatches
(
  CompanyID int PRIMARY KEY,
  DunsID int
)

When you're done with #newMatches, you should get rid of it so you can create it again later (temp tables are connection scoped!!)

DROP TABLE #newMatches
David B
Also consider using variable tables, as they are scoped as variables. DECLARE @newMatches TABLE(CompanyId int PRIMARY KEY, DunsId int)
David B
+2  A: 

I think David and Cervo combined have hit on the problem here.

I'm pretty sure part of what was happening was that we were using #newMatches in multiple triggers. When one trigger changed some rows, it would fire another trigger, which would attempt to use the connection scoped #newMatches.

As a result, it would try to, find the table already existed with a different schema, die, and produce the message above. One piece of evidence that would be in favor: Does inserted use a stack style scope (nested triggers have their own inserteds?)

Still speculating though - at least things seem to be working now!

Chris
Nested triggers each have their own inserted magic table.
David B
A: 

Trigger code (because it must run everytime the data is updated) must be efficient and must account for multiple record inserts. You've succeeded at the second but not the first. You have made this overly complicated and have used things such as Not in statements that are usually less efficeint than using a left join. Temp tables are unnecessary here (I would never consider using one in a trigger) as they add to the inefficiency of the trigger. There is not reason not to write From inserted i instead of FROM (select dunsId, name from #magic) i

The first is likely to be faster and is simpler to read and maintain.

Here: JOIN ( select case when charindex('/',url) <> 0 then left(url, charindex('/',url)-1) else url end urlMatch, * from companies ) c ON dandb.url = c.urlMatch

You are selecting all the fields in the table even though you only appear to be using one. Why? You are also running that case stament on all the records in company even though after you join you may not need all of them.

Also in general I would avoid using select * but especially in a trigger. Suppose you are inserting into another table and you used select * from some table joined to inserted or deleted. Adding a column to that table would cause the trigger to fail and stop all data changes until it was fixed.

You've also used a function in the trigger. This coudl be painfully slow if you havea large insert. I suggest you test this by updating a large group of records and see what happens. All data changes do not happen just from the user interface, one record at a time. There will be times when one field is updated from an ad-hoc query in management studio (when all prices need to be adjusted by 10% as the simplest example that comes to mind.) Your trigger needs to be able to handle those types if updates as well as the ones you are expecting. I would run a test case updating 100000 rows and see how much this trigger slows things down.

Maybe this isn't really answering your problem, but the trigger just is so far from optimal, I had to say it.

HLGEM
Actually the temp tables are gone. I don't know what the extra columns were for, so they're gone too.We eventually have to run the function on every row. We actually got a good metric of performance because we do bulk loading. I may disable the trigger, if using a procedure for bulk is faster...
Chris
Would doing an UPDATE after the bulk load be faster than the equivalent trigger?
Chris
A: 

I imagine that would depend somewhat on how the trigger is written. If the trigger loops through the records in inserted, almost certainly yes.

HLGEM