This blog post is written in response to the post showing some of the worst practices of past. Well, just like last month’s theme, everybody learns by doing it one step at a time. In my case, I started my career as a network engineer and had no database knowledge during that time. I can still remember my old code which became quite a laughingstock when it was sent for a code review. This story is indeed interesting, so instead of writing shortly, I am going to write today in detail.
It happened about 8 years ago when I was working as Network Engineer in United States. I was responsible for a large data center and we had more than 40 servers under my watch. In our SQL Server database we had one table for the inventory of all the servers. On that table we used to keep a lot of details of the server. To keep it simple, I will say that we have only two columns in that table: ServerID and ServerName. I was asked to write a script which was intended to retrieve all the data from this table and insert them into the new table that was created by the database administrator. I thought I could handle SQL Server script well enough because I learned SQL a bit when I was younger. I ended up writing a script where I wrote cursor to SELECT single row from the table and insert into new table. Well, this went very fine. The script was taking a while to perform its tasks; nevertheless, it worked. I used this script for a short while.
Here is the script that I wrote. I am using sample and simple database for example. Please note that this is not a good practice.
-- Create Table and Populate with Sample Data
CREATE TABLE ServerTable (ServerID INT, ServerName VARCHAR(100))
INSERT INTO ServerTable (ServerID, ServerName)
SELECT 1, 'First Server'
UNION ALL
SELECT 2, 'Second Server'
UNION ALL
SELECT 3, 'Third Server'
-- Creating New Table
CREATE TABLE NewServerTable (ServerID INT, ServerName VARCHAR(100))
-- Insert Logic
DECLARE @Flag INT
SELECT @Flag = COUNT(*) FROM ServerTable
WHILE(@Flag > 0)
BEGIN
INSERT INTO NewServerTable (ServerID, ServerName)
SELECT ServerID, ServerName
FROM ServerTable
WHERE ServerID = @Flag
SET @Flag = @Flag - 1
END
SELECT ServerID, ServerName
FROM NewServerTable
-- Clean up
DROP TABLE ServerTable
DROP TABLE NewServerTable
This script worked very well till we had changed in company policy. We then had a newly- appointed CTO (which I was about to become 3 years after this incidence), who introduced the requirement of Code Reviewing of all both old codes and new ongoing codes. When my code went to a code review, my Team Leader really laughed at my code. So he sent me new piece of the code which was much shorter and more efficient. The code which he proposed was as follows. I honestly say this was much better than my code.
-- Create Table and Populate with Sample Data
CREATE TABLE ServerTable (ServerID INT, ServerName VARCHAR(100))
INSERT INTO ServerTable (ServerID, ServerName)
SELECT 1, 'First Server'
UNION ALL
SELECT 2, 'Second Server'
UNION ALL
SELECT 3, 'Third Server'
-- Creating New Table
CREATE TABLE NewServerTable (ServerID INT, ServerName VARCHAR(100))
-- Insert Logic
INSERT INTO NewServerTable (ServerID, ServerName)
SELECT ServerID, ServerName
FROM ServerTable
SELECT ServerID, ServerName
FROM NewServerTable
-- Clean up
DROP TABLE ServerTable
DROP TABLE NewServerTable
If you’ve noticed, the new insert code is just reduced to two lines only. Today you might think it is a very simple code, but for me it was a very new thing during those times. Now comes the best part of this whole story.
My team leader, who was very supportive and extremely positive person, asked me to demonstrate this whole incidence on our next team meeting. Honestly, I did not feel bad about accepting my mistake and learning new things. During the team meeting, I first accepted the fact that I was wrong with my code and thanked the Team Leader for giving me the opportunity to improve and develop myself. Because of his encouragement to accept my own mistake and improve from that incident, today I have reached this place.
Please get rid of cursors and use INSERT INTO…SELECT or SELECT INTO logic.
Reference:Â Pinal Dave (https://blog.sqlauthority.com)
37 Comments. Leave new
Its like you learn my thoughts! You seem to grasp a lot about this,
like you wrote the book in it or something. I think that
you could do with a few p.c. to force the message house a little bit, but other
than that, this is wonderful blog. A great read.
I will definitely be back.
hello!,I really like your writing so much! share we keep in touch more about your post on AOL?
I require an expert in this space to resolve my problem.
Maybe that’s you! Taking a look ahead to see you.
thank you for your small but important lessons
Thanks a lot for sharing this with all of us you really know what you are talking about!
Bookmarked. Please also visit my website =).
We could have a link exchange arrangement between us!
SELECT .. INTO and INSERT INTO … SELECT * are both bulk logged operations and can benefit from it if the recovery model is SIMPLE or BULK LOGGED
BTW: Pinal, great blog.
I’ve learned vey much from it within the last 5 years :)
Though these are fast but create table lock. So in highly concurrent scenarios these does not scale. In scenario where high concurrent insert with bigger row set create problem if use build insert. In these scenarios cursor bases operation is better. So build operation is not always good. Any inputs?
Hi Pinal,
Need your guidance in solving below query
-1. Current process has ONE MS Access Database table with all fields of information in it. Lets say STUDENTS ( has all info about students bunched togather)
-2. Designed a new SQL server 2008 DB with all info grouped togather in resp tables- so there are more than one table now – Student, Personal, Courses, Teachers, Grades, Research
-3. Need to migrate data from ONE old table and distribute it over into these multiple tables with ‘Student_id’ as primary key of Students table, and ‘Student_id’ – ref as Foreign Key in all other tables
– But some columns from records of old table has data which needs to be checked for before inserting into new tables ( some data has been added to Static data – Such as Course list, so sourse name has to be one othe entires from COURSE_NAME table, while old data has some free text data inti so insert would fial on FK contraint)
What is the best way to achieve this data migration into new table stucture?
Thanks for (any) pointers
Say i am inserting 50 rows using insert… select , if any error occurs will it roll back automatically?
Hi Pinal, first I would like to thank you for such an excellent site. I have learned many things from you concerning SQL server. Right now I am honestly getting confused by cursor performance.
Everywhere I look I see that Cursors are bad… I am trying to reconcile these statements with my testing results against a foreach loop in SSIS 2012. Any clarity or if I am doing something incorrectly in SSIS would be appreciated.
This was my test cursor with 1000 records.
/************ Test Cursor Performance ********************/
DELETE testImportDatabaseMaintenance
DECLARE @DBMRecordid AS INT , @Endpointid AS VARCHAR(50)
DECLARE test_cur CURSOR FAST_FORWARD READ_ONLY FOR
SELECT idm.DBMRecId ,idm.EndpointId
from ImportDatabaseMaintenance idm
OPEN test_cur
FETCH NEXT FROM test_cur INTO @DBMRecordid , @Endpointid
WHILE @@FETCH_STATUS = 0
BEGIN
INSERT Testimportdatabasemaintenance
(testImportDatabaseMaintenance.DBMRecId , testImportDatabaseMaintenance.EndpointId )
VALUES (@DBMRecordid , @Endpointid )
FETCH NEXT FROM test_cur INTO @DBMRecordid , @Endpointid
END
CLOSE test_cur
DEALLOCATE test_cur
/*****************************************************************************************/
this cursor completed in less than 1 second from SSMS.
I used this sp with a foreach loop in SSIS.
ALTER PROCEDURE A_Test_DBM_SSIS_3 @DBMRecId AS INT ,
@EndPointId as varchar(50)
AS
INSERT testImportDatabaseMaintenance
(
DBMRecId,
EndpointId
)
VALUES ( @DBMRecId , @Endpointid)
GO
The package does the same functionality as the cursor.
in a container I load the variable objIdbm with the recordset and then execute a foreach ADO Enumerator against the Stored Procedure referenced above. time to complete 388 seconds.
I started to search the web and found many other blogs and articles that are basically having the same results.
Am I missing something?
Thank you in advance for your time.
thanks for sharing, it is a good article
You are welcome.