is this stored procedure correct?

Question

I have a stored procedure below, and I don't know if it's correct.

I am inserting records in table PlanFinder.InvalidAwps, and deleting the same records from the PlanFinder.NdcAwp table.

Also, can anybody help me with adding try catch in the same stored procedure?

Alter procedure PlanFinder.InsertInvalidRecords 
as 

Truncate table [PlanFinder].[InvalidAwps]  

INSERT INTO [PlanFinder].[InvalidAwps](Ndc, AwpUnitCost)      
   SELECT DISTINCT P.Ndc Ndc, A.Price AwpUnitCost  
     FROM PlanFinder.PlanFinder.HpmsFormulary P  
LEFT JOIN (SELECT Ndc, Price 
             FROM MHSQL01D.Drug.FdbPricing.vNdcPrices   
            WHERE PriceTypeCode = '01' 
              AND CurrentFlag = 1) A ON P.Ndc = A.Ndc   
    WHERE (   A.Ndc IS NULL 
           OR A.Price <= 0 
           OR A.Price IS NULL)  
      AND p.Ndc IS NOT NULL 

DELETE FROM PlanFinder.NdcAwp 
WHERE Ndc IN (SELECT Ndc 
                FROM PlanFinder.InvalidAwps)

Answer 1

First, make sure your linked servers are set up.

Here is an overview from MS on linking servers.
Here is an article on the actual syntax of the command.

It looks like the proc will run after your edits (removing the GO) but it's impossible to know for sure without knowing what your tables look like - what columns are in what tables and how they relate.

Some efficiency things to think about:

• IN is not a very efficient operator. You can almost always get better performance from using EXISTS instead.
• You have multiple WHERE clauses with OR's...this is also going to be pretty inefficient (since SQL Server will be checking each for for each condition). Do you need to allow 'NULL' in your price column? Do you need to allow NULL in your NDC column? Remember NULL is different from blank or 0.... If you remove your nullability from the NDC and price fields, your 4 operations become 1 operation (PRICE <= 0).

Answer 2

Your question is still a bit unclear. "Is this stored procedure correct?" is rather vague, and you also asked for some help adding in "Try ... Catch" logic.

First, what kind of "correct" are you looking for? If "valid sql" is all you're looking for, then assuming your linked server MHSQL01D is set up correctly and all of the object names are valid, yes, it should work. You're allowed to truncate, insert, and delete all within that procedure.

There are other issues, such as performance of "IN". Also, you're joining to a table on a linked server and actually returning (potentially, depending on how many valid cases there are) a lot of data that you simply throw away (all of the data from vNdcPrices where Price > 0). I assume that the invalid cases are the smaller set, so you might be able to re-write it such that the smaller data set is what's sent over the wire.

As far as the other question, what are you trying to catch? An exception when procedure is executed but there's an issue on the linked server side (linked server down, table gone, etc) or an exception when the procedure is created? The linked server will need to be up and the table needs to be up and all names valid at create / alter time. If you want to catch issues at execute time, the following will work:

BEGIN TRY
    INSERT ... LinkedServer.DB.Schema.Table ...
END TRY
BEGIN CATCH
    -- Error Handling Code --
    ...
END CATCH