December 19, 2007 at 6:50 am
Heh... feed the vendor a nice juicy pork-chop... with a sling-shot! 😉
--Jeff Moden
December 19, 2007 at 6:52 am
Jeff Moden (12/19/2007)
Heh... feed the vendor a nice juicy pork-chop... with a sling-shot! 😉
ROFL! That will at least get their attention.
December 19, 2007 at 6:52 am
Brett,
Obviously, a couple of us have run into the problems associated with "Sequence Tables" such as you've described... if you would, please, generate the table creation for your sequence table and check out the following URL to generate some data we can test with, please...
http://qa.sqlservercentral.com/articles/Best+Practices/61537/
--Jeff Moden
December 20, 2007 at 5:48 am
-- there may be loss of formatting
-- I've used 4 sapce tab
-- firstly I don't like loops but do prefer them over cursors
-- for data movement set based solutions are what you should be trying to do
-- below is response to you question how do I do this while loop
-- personally I would not use a loop unless I had to
SET NOCOUNT ON;
DECLARE -- one declare it less typing
@PHONEID INT
,@PERSONID INT
,@PERSLEGACYACCOUNTID INT -- for loop
,@PHONETYPE VARCHAR(40) -- for loop
,@PHONENUMBER VARCHAR(20) -- for loop
,@LoopCount int -- for loop
,@BatchDate datetime -- for loop
-- Create # table to act as working list
-- Would normally create this with a CREATE TABLE statement but do not have the details
-- if smallish record set and particulary if X64 machine consider a Table variable here - this will be in memory
SELECT
PHON_TYPE
,LEFT(PHON_NUMBER,20) AS PHON_NUMBER
,PHON_PERSLEGACYACCOUNTID
INTO #loopList
FROM
dbo.ZZIMPPHONE a
WHERE
EXISTS (SELECT 1 FROM dbo.PERSON z WHERE a.PHON_PERSLEGACYACCOUNTID = b.PERS_LEGACYACCOUNTID);
-- this would normally be part of the create table statement
ALTER TABLE #loopList
ADD RowID int IDENTITY (1,1);
--Add an index on RowID if a large data set and it would benefit from an index
-- Prepare cycle count -- prefer this method over deleting the records and using select top 1 less I/O
-- always use SELECT over SET it far more efficient at allocating values to variables and you can set more than one value at a time
SELECT
@LoopCount = 0
,@BatchDate = GETDATE() -- more efficient to get the value once
--While loop - use exists because it most efficient for checking if one or more records exists
WHILE EXISTS(SELECT 1 FROM #loopList WHERE RowID > @LoopCount)
BEGIN
SELECT @LoopCount = @LoopCount + 1 -- to pick the next record in the list
-- get working values
SELECT
@PHONETYPE = PHON_TYPE
,@PHONENUMBER = PHON_NUMBER
,@PERSLEGACYACCOUNTID = PHON_PERSLEGACYACCOUNTID
FROM
#loopList
WHERE
RowID = @LoopCount
-- In the next line of code
-- Are you getting a unique sequence value that you are going to use to insert into the table?
-- Or are you getting the same value every time?
-- If it is the latter, I believe there is no need for a loop at all. But I would need more info to show you why.
EXEC @PHONEID = dbo.eware_get_identity_id 'Phone';
SELECT -- don't like top 1 you cannot guarantee the same result every time and it is not efficient approach
@PERSONID = (SELECT TOP 1 PERS_PERSONID FROM PERSON WHERE PERS_LEGACYACCOUNTID = @PERSLEGACYACCOUNTID);
-- this should be a proc that you are calling so your code is modular
INSERT INTO dbo.Phone
(
Phon_PhoneId,
,Phon_PersonID
,Phon_Type
,Phon_Number
,Phon_CreatedBy
,Phon_CreatedDate
,Phon_UpdatedBy
,Phon_UpdatedDate
,Phon_TimeStamp
)
SELECT -- use SELECT as it allows me see data when troubleshooting I always lable as it helps the reader (efficiency should stop at the code performance)
@PHONEID AS Phon_PhoneId
,@PERSONID AS Phon_PersonID
,@PHONETYPE AS Phon_Type
,@PHONENUMBER AS Phon_Number
,1 AS Phon_CreatedBy
,@BatchDate AS Phon_CreatedDate
,1 AS Phon_UpdatedBy
,@BatchDate AS Phon_UpdatedDate
,@BatchDate AS Phon_TimeStamp;
END
-- don't forget to drop your temp table
DROP TABLE #loopList
/*
I don't concur with others in the use of a function as this is not efficient and should be only followed if
this the optimal way to achieve the result. If you are not shore then I suggest you avoid if you have
not got time to investigate multiple solutions.
I recently re-wrote someone's code that had used functions in the select list columns and
reduced its execution time from over 10 minutes to less than 3. The reason lies in the way the SQL Server
engine builds the execution plan. The performance issue lies in the number of time the function is called
or whether it can use an index. It is even worse if the function is porely writen.
Also avoid them in where clauses or joins. There is a time and palce for everything and this is a subject that we could spend pages discussing.
*/
I hope you found this useful
December 20, 2007 at 11:03 am
Basically it is a proprietary SP provided by a SQL based web application for the purpose of generating unique integer based row IDs. It does several things including calling other SP’s for record locking, remote client id range updates as well index updates… so I don’t think so, but here is the code nonetheless:
Oh my... I missed that large chunk of code... I also have to say that's gotta be the worst "GetNextID" proc I've ever seen.
I don't know if I'll have the time to write a decent code example for a replacement to be used by your procs, Brett, but I'll try to make some.
--Jeff Moden
December 21, 2007 at 12:21 pm
Wow guys thanks so much for everyones help.
In regards to the questions around get_next_CRM_ID, I dont really have a level of skills that would make me feel comfortable bypassing and/or optimising it. Aside from that, the vendor has explicitly said that not using it is unsupported.
That being said, if someone feels like having a stab at optimising the procedure or writing insert procedures as per the original topic that can use a more efficient alternative, I'd be happy to take it on board and maybe even put forward some suggestions to the vendor - due to our current status with the vendor we have a considerable amount of input in to these kind of things so may be able to make a difference.
In terms of the original script, I unfortunately haven't noticed any significant difference worth writing home about in terms of execution time after changing it from a cursor to a while loop, but at least it has definitely opened some doors for me in terms of dealing with some other similar situations.
Next challenge for me is getting my head around how to do my code in a set based fashion instead of row by row... but hey, everyone has to start somewhere right?
And compliments of the season to you all!!
Brett
December 21, 2007 at 12:41 pm
In terms of the original script, I unfortunately haven't noticed any significant difference worth writing home about in terms of execution time after changing it from a cursor to a while loop, but at least it has definitely opened some doors for me in terms of dealing with some other similar situations.
Heh... that's because a cursor isn't much more than a Temp table with a While loop... the cursor uses slightly more resources so it's a bit slower, but replacing cursors with While loops is a bit like spitting into a strong wind. 😀
Next challenge for me is getting my head around how to do my code in a set based fashion instead of row by row... but hey, everyone has to start somewhere right?
The only way you could do that is to have the vendor make the NextID code so that you can "reserve" groups of ID's... I think they have the basic structure to allow that but we'd need more info on how to reserve by "increment".
They do a heck of a lot of unnecessary locking... like I said, I'll see if I can come up with a working example of how sequence tables should be incremented.
--Jeff Moden
December 21, 2007 at 1:07 pm
Jeff said: "They do a heck of a lot of unnecessary locking... like I said, I'll see if I can come up with a working example of how sequence tables should be incremented."
One thing that puzzles me is the range of ids reserved for each table. It appears that they all dip from the same pool of numbers instead of each table being encapsulated beginning with 1 and going to n.
If there were not that reserved range business one could dump the records to be added to a temp table with an Identity column whose seed is the last id used + 1. That would be the next phoneid. From there the data could be inserted into the Phone table en mass and the last id used table would be updated. Looks to me like they are going around the world for something that should be pretty straightforward. Of course, that's looking at it from the outside.
December 21, 2007 at 1:15 pm
The seed should probably the value returned from the NextID proc...
Other problem is, you have to increment the NextID in the NextIDtable by the number of rows you have in the temp table... in order to keep from messing up the works when multiple connections call the NextID proc, you must provide the increment at that time or deadlocks are sure to occur.
--Jeff Moden
December 21, 2007 at 1:53 pm
Jeff Moden (12/21/2007)
The seed should probably the value returned from the NextID proc...Other problem is, you have to increment the NextID in the NextIDtable by the number of rows you have in the temp table... in order to keep from messing up the works when multiple connections call the NextID proc, you must provide the increment at that time or deadlocks are sure to occur.
You are right, I hadn't thought about multiple connections.
Assuming the eware_get_identity_id sproc can be updated: get the number of records to be inserted, then pass the record count to the eware_get_identity_id sproc along with the table name. Eware_get_identity_id gets the current value and locks the table then updates it to that value + record count passed in and returns the starting value which is then used as the seed value for Identity. Only one lock to the NextID per group of records to be inserted instead of one per RBAR. The calculation of the phoneid is then done as a set op and the insert performed as one transaction.
This of course ignores the whole range-of-values thing. If that has to be maintained things get tricker. Might have to create a Values table ... Hmm, requires more thought.
I'm working on this in my head and I have a feeling I've overlooked something else. When I get the chance to hammer at it in SSMS I'll see how it works.
"I'd make a mental note but I don't have anything to write on."
December 21, 2007 at 7:08 pm
Jeff Moden (12/21/2007)
The only way you could do that is to have the vendor make the NextID code so that you can "reserve" groups of ID's... I think they have the basic structure to allow that but we'd need more info on how to reserve by "increment".
They do a heck of a lot of unnecessary locking... like I said, I'll see if I can come up with a working example of how sequence tables should be incremented.
That's what I thought, but then again - I would lock things too if I had a process run for 20 minutes adding records. Like you said - it's sad to see - but it's all related.
----------------------------------------------------------------------------------
Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?
Viewing 11 posts - 16 through 25 (of 25 total)
You must be logged in to reply to this topic. Login to reply