Create SQL Trigger to run script 2

[kpetree10]

Hi All, I'd like to run the following script...

CREATE TABLE tbl 
(Ord_no Char(8), shipment Char(30))
insert into tbl SELECT DISTINCT Ord_no AS Ord_no, Shipment AS Shipment from wspikpak  where shipped='Y' AND  (DATEDIFF([day], ship_dt, GETDATE()) = 0)
update o set O.user_def_fld_5=T.shipment
from tbl T  JOIN oeordhdr_sql O
on T.ord_no =O.ord_no
where T.ord_no=O.ord_no
drop table tbl

Whenever the Shipped field on a record in the table wspikpak is changed from N to Y. I figured the best way to do this is through a trigger but I've never set one up before and was hoping someone here could give me a hand. Thanks in advance!

 

[djj55]

CREATE TRIGGER myTrigger ON wspikpak 
AFTER UPDATE
AS 
IF UPDATED(shipped) 
BEGIN 
    CREATE TABLE tbl (
        Ord_no Char(8), 
        shipment Char(30))

    insert into tbl 
    SELECT DISTINCT 
        Ord_no AS Ord_no, 
        Shipment AS Shipment 
    from wspikpak  
    where shipped='Y' 
        AND  (DATEDIFF([day], ship_dt, GETDATE()) = 0)

    update o 
    set O.user_def_fld_5=T.shipment 
    from tbl T 
    JOIN oeordhdr_sql O 
    on T.ord_no = O.ord_no 
    where T.ord_no=O.ord_no
    drop table tbl
END

Not tested.

djj
The Lord is my shepherd (Psalm 23) - I need someone to lead me!

 

[djj55]

Sorry hit wrong button.

I would read BOL for CREATE TRIGGER.

You can also use the inserted and deleted tables

I personally use a different style of programming so I did not rework your code.

djj
The Lord is my shepherd (Psalm 23) - I need someone to lead me!

 

[kpetree10]

That worked! The only change I had to make was that UPDATED has to be UPDATE for MS SQL. Thank you!

 

[gmmastros]

kpetree10,

I was looking at your original code. There is a better way that this can be written. The problem here is that you are creating a real table, using it, and then dropping it. It is entirely possible that this trigger could fire multiple times simultaneously which would cause one to fail because the real table already exists.

Your code can be re-written so that it does not depend on a real table to store temporary data.

I am not in the habit of offering advice when it's not asked for. Therefore, if you would like comments on your original code, I will gladly respond. If you are satisfied with it, then please ignore this post.


-George
Microsoft SQL Server MVP
My Blogs
SQLCop
twitter
"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[kpetree10]

Sure, I'd appreciate the help. I actually didn't write the script, our former IT Manager wrote it, I just kept using it. I can see how it could cause a problem and it definitely would be based on how the data is.

 

[gmmastros]

Good. I'm glad that you are willing to take another look at this.

The first, and most important thing here is to remove the part about the real table. This is relatively simple to do once you know how. The query I show below should be identical to the code you originally had, but without the real table.

The method I show below is called a "derived table" approach. Looking at your original code, you create a table, load it with distinct data, use it to join with the oeordhdr table, and then drop the table. The query you use to fill the real table is:

       SELECT DISTINCT Ord_no AS Ord_no, 
              Shipment AS Shipment 
       from   wspikpak  
       where  shipped='Y' 
              AND  (DATEDIFF([day], ship_dt, GETDATE()) = 0)

We can use a derived table approach to eliminate the need for the real table. When you use a derived table, you basically enclose it in parenthesis and give it an alias. Outside the parenthesis, you can treat the derived table as though it were a real table, but you must consistently use the alias you give it.

The derived table query would look something like this:

update o 
set    O.user_def_fld_5=[blue]T.[/blue]shipment
from   [!]([/!]
       Your Derived table query here
       [!])[/!] [blue]As T  [/blue]
       JOIN oeordhdr_sql O
         on [blue]T.[/blue]ord_no =O.ord_no

First, notice the red parenthesis. This makes the query within the parenthesis a derived table. Next, notice the blue parts. The "As T" is the alias for the derived table, and the alias is used consistently throughout other parts of the outer query. In this case, it's used in the select clause and the join clause. You could also use it in a where clause and/or order by clause too.

Putting it all together:

update o 
set    O.user_def_fld_5=T.shipment
from   (
       SELECT DISTINCT Ord_no AS Ord_no, 
              Shipment AS Shipment 
       from   wspikpak  
       where  shipped='Y' 
              AND  (DATEDIFF([day], ship_dt, GETDATE()) = 0)
       ) As T  
       JOIN oeordhdr_sql O
         on T.ord_no =O.ord_no

Your original query had a where clause, (where T.ord_no=rd_no). This was unnecessary because you are inner joining to the oeordhdr table on the same criteria. It didn't hurt to have it there because the query optimizer was smart enough to remove it for the execution plan, but it's basically redundant code.

I have more comments to make, but I'd like to make sure you understand this part first. Does this make sense?

-George
Microsoft SQL Server MVP
My Blogs
SQLCop
twitter
"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[kpetree10]

That makes perfect sense! Please go on.

 

[gmmastros]

The next comment I have concerns your derived table query; specifically the where clause.

SELECT DISTINCT Ord_no AS Ord_no, 
       Shipment AS Shipment 
from   wspikpak  
where  shipped='Y' 
       AND  (DATEDIFF([day], ship_dt, GETDATE()) = 0)

If I had to guess, I would say that shipped is a status field, with very few statuses available. There may be Y, N, H, etc.. but probably not more than a dozen statuses. As such, this is not a very good column to have an index on because of the selectivity of the index. I don't really have a problem with this part.

The problem I have is with the 2nd condition on ship_dt. There is a concept that you should be aware of when writing queries called SARGABLE.

Basically, you can have multiple indexes on a table. Depending on your query, SQL Server may use an index to speed up certain operations. When you use a function on a column, like you are doing here with the DateDiff function, SQL Server is unable to use an index to speed up the query. Essentially, SQL Server will have to scan every row in this table and apply the function to the data in the row. It would be relatively simple to re-write the query in a way that SQL Server would be able to use an index (if it exists) on the table.

What I would like you to do is run the following queries in a SQL Server Management Studio query window.

Select * From sys.sysobjects where id = 20

Select * From sys.sysobjects where id + 0 = 20

On your keyboard, press CTRL-M, and then F5 to run the query. Each query will return the exact same data. You should see an "Execution Plan" tab within SQL Server Management Studio. When you click on the execution tab, you will see both queries and a graphical display showing the steps that the query optimizer uses to execute the query. When I run this, I see

[tt]
Query 1: Query cost (relative to the batch): 11%
Query 2: Query cost (relative to the batch): 89%
[/tt]

The percentages you see will add up to 100. But, the important part here is to notice that the first query took 11% of the total time and the second took 89%. Since both queries return the same data, it is obviously better to use the one that is faster, right?

So... how do we re-write your query so that it can effectively use an index? Like this:

SELECT DISTINCT Ord_no AS Ord_no, 
       Shipment AS Shipment 
from   wspikpak  
where  shipped='Y' 
       AND  ship_dt = DateAdd(Day, DateDiff(Day, 0, GetDate()), 0)

If you run both versions of this code within the same query window and then look at the execution plan, you should see that the new one executes faster. If it doesn't, then there is probably not an index that has ship_dt as the first column.

Please note that you may not notice a difference in performance. It all depends on the indexes that are available and the amount of data in the table. With small tables (few number or rows), everything is going to be fast.

Make sense?

-George
Microsoft SQL Server MVP
My Blogs
SQLCop
twitter
"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[kpetree10]

I ran my select statements with Execution plan turned on and here's what I got...

What I ran...

SELECT DISTINCT Ord_no AS Ord_no, 
              Shipment AS Shipment 
       from   wspikpak  
       where  shipped='Y' 
              AND  ship_dt = DATEADD(Day, DateDiff(Day, 0, GetDate()),0)

       SELECT DISTINCT Ord_no AS Ord_no, 
              Shipment AS Shipment 
       from   wspikpak  
where  shipped='Y' 
              AND  (DATEDIFF([day], ship_dt, GETDATE()) = 0)

and the results...

Query 1: Query cost (relative to the batch): 49%
Query 2: Query cost (relative to the batch): 51%

So if I did it right it did make a slight difference.

Either way that information is great to know for more complicated scripts. I greatly appreciate you taking the time to explain it all to me.

 

[gmmastros]

Just curious...

How long does it take to run the query?
How many total rows are in the wspipak table?
What do the indexes look like in the wspipak table?

sp_helpindex 'wspipak'

Like I said, if there are no appropriate indexes on the wspipak table, then sargable queries won't help.



-George
Microsoft SQL Server MVP
My Blogs
SQLCop
twitter
"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom