Personally I would choose to pass a list of id's in as a table parameter to the stored procedure this would then allow you to do a set-based update instead of a row by row one which is less efficient.
I have never personally used the EF but a good artical on performing the above using ADO is below (ignore the fact it says it is for SQL 2008 as it also works on 2005). The same strategy would work better for you in this situation but you may need to adapt the implementation based on the fact you are using the Entity Framework.
http://www.mssqltips.com/sqlservertip/2112/table-value-parameters-in-sql-server-2008-and-net-c/
EDIT
As you rightly pointed out I am wrong about the fact this works on 2005 - sorry about that!
However, I have some alternate suggestions.
As SQL Server 2005 does support table variables (just not as parameters to stored procedures as you pointed out) you could parse the delimited string and insert the id's into a table variable. You could then use the table variable to perform a set-based update.
Alternatively the link below provides a different take on the same problem by persisting the values to a table first thereby avoiding serialization and de-serialization of the id values:
http://weblogs.sqlteam.com/jeffs/archive/2007/06/26/passing-an-array-or-table-parameter-to-a-stored-procedure.aspx
I hope this helps you.
From what I can see you have two problems. The first is the doubling. At a guess it is because of the SELECT
statement in your INSERT
statement.
SELECT @NewMetricID,
NULL,
sm.sortorder,
ISNULL(sm.MetricOrder, 1),
ISNULL(sm.CategoryOrder, 1),
sm.RptCurrentGroup,
'System',
GetDate(),
NULL,
NULL,
'N',
NULL,
NULL
FROM Metric_Instance mi
INNER JOIN Shared_Metrics sm on mi.MetricID = sm.MetricID
WHERE mi.MetricID = @CurrentMetricID
AND mi.MetricDisposition <> 'Suspended'
AND mi.isDeleted <> 'Y'
AND sm.isDeleted <> 'Y';
I would be willing to bet your query is returning more than one row. Any time you have a join like that you run a risk of creating duplicate rows. The easy way to fix it is to do this:
SELECT DISTINCT @NewMetricID,
NULL,
sm.sortorder,
ISNULL(sm.MetricOrder, 1),
...
...
...
The only real change is that that I'm putting a DISTINCT
at the top of the query. This means that if there are any VALID
duplicates you are still going to pick them up. You are not actually referencing any columns from Metric_Instance so you could change it from a JOIN
to a WHERE EXISTS (SELECT 1 ...
On to problem two. In your code you are using scope_identity()
. This is only going to return the last identity value created. If you insert more than one row the previous rows are going to be missed. The best solution here is to get rid of the cursor and change this into a batch process. INSERT
all possible rows initially using the OUTPUT clause to dump a list of identity values and CurrentMetricIDs into a temp table. Then do your update using that temp table to update all of the values at once. Not only will this fix your problem but will probably run quite a bit faster as well.
Best Answer
You could use Table-Valued Parameters to pass a set of
InvoiceItems
to a procedure that will insert a newInvoice
and also insert theInvoiceItems
to theInvoiceItems
table.Without knowing your table schema, I am just going to throw in a few columns.
Example user-defined table type for use as a table-valued parameter:
Example procedure using
scope_identity()
to get theInvoiceId
after inserting a row intodbo.Invoice
:Table-valued parameter reference:
In SQL Server 2012+, an alternative to using an auto-numbered
identity()
forInvoiceId,
you could use asequence
.Example of how to create and use a
sequence
as the primary key ondbo.Invoice
:In this example, instead of using
scope_identity()
after inserting a row intodbo.Invoice
, we would usenext value for
dbo.InvoiceIdSequence
before inserting a row intodbo.Invoice
.Sequence reference:
sequence
basics - Joe Celkosequence
- msdnnext value for
- msdn