• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 977
  • Last Modified:

How do I create a loop to insert all rows of one table to another with stored procedure

I written this stored procedure to populate my Traps table but the procedure only populates one row multiple times.  I can't seem to get the procedure to insert the correct rows via a loop.  How can I tweak my script to insert the rows via a loop?
if exists (select * from sysobjects where name = N'dbm_HostName2Trap')
 
   drop procedure dbm_HostName2Trap
go
 
CREATE PROCEDURE  dbm_HostName2Trap
AS
 
SET NOCOUNT OFF
-- declare variables
Declare @T_ID int
Declare @IP_Address varchar(15)
Declare @Hostname varchar(255)
Declare @N_ID int
Declare @iLoop int
Declare @iNextRowId int
Declare @iCurrentRowId int
 
-- initialize variables 
SET IDENTITY_INSERT dbo.Traps ON
SELECT @T_ID = (MAX(TRAPID) + 1) FROM Traps;
SELECT @Hostname = Hostname from IP_Resolve;
SELECT @IP_Address = IP_Address from IP_Resolve;
Select @N_ID = NodeID from Nodes,IP_Resolve where IP_Resolve.IP_Address = Nodes.IP_Address;
SELECT @iLoop = 1;
SELECT @iNextRowId = MAX(ID) FROM IP_Resolve;
 
IF ISNULL(@iNextRowId,0) = 0
 
   BEGIN
            SELECT 'No data in found in table!'
            RETURN
   END
 
-- Retrieve the first one
select @iCurrentRowId = ID from IP_Resolve where ID = @iNextRowId;
-- insert statement
INSERT INTO dbo.Traps (TrapID,EngineID,DateTime,IPAddress,Community,Tag,Acknowledged,Hostname,NodeID,TrapType,ColorCode) VALUES ( @T_ID,'2',DEFAULT,@IP_Address,'public',DEFAULT,DEFAULT,@Hostname,@N_ID,DEFAULT,DEFAULT ); 
 
-- start the main processing loop.
 
WHILE @iLoop = 1
 
   BEGIN
 
     -- This is where you perform your detailed row-by-row
 
     -- processing.     
 
     -- Reset looping variables.            
 
            SELECT   @iNextRowId = NULL            
 
            -- get the next iRowId
SELECT @T_ID = (MAX(TRAPID) + 1) FROM Traps;
 
            SELECT   @iNextRowId = MAX(ID) FROM IP_Resolve
            WHERE    ID < @iCurrentRowId
 
            -- did we get a valid next row id?
 
            IF ISNULL(@iNextRowId,0) = 0
 
               BEGIN
 
                        BREAK
 
               END
 
            -- get the next row.
select @iCurrentRowId = ID from IP_Resolve where ID=@iNextRowId;
 
      INSERT INTO dbo.Traps (TrapID,EngineID,DateTime,IPAddress,Community,Tag,Acknowledged,Hostname,NodeID,TrapType,ColorCode) VALUES ( @T_ID,'2',DEFAULT,@IP_Address,'public',DEFAULT,DEFAULT,@Hostname,@N_ID,DEFAULT,DEFAULT ); 
       
   END
RETURN

Open in new window

0
cybersapp
Asked:
cybersapp
  • 9
  • 7
  • 3
  • +3
3 Solutions
 
JestersGrindCommented:
I would avoid looping altogether.  SQL is much better at performing set-based operations than looping.  You also don't have to insert anything into the identity column.  SQL will do that for you.  Something like this:

INSERT INTO dbo.Traps (TrapID,EngineID,DateTime,IPAddress,Community,Tag,Acknowledged,Hostname,NodeID,TrapType,ColorCode)
SELECT '2', DEFAULT, IP_Address,'public',DEFAULT,DEFAULT,Hostname,@N_ID,DEFAULT,DEFAULT
FROM IP_Resolve


Greg


0
 
k_murli_krishnaCommented:
You need to declare a cursor and loop though it. Example stored procedure with cursor is at:
http://www.eggheadcafe.com/PrintSearchContent.asp?LINKID=141
0
 
k_murli_krishnaCommented:
Greg may be correct but only for a single row. If we have to deal with multiple rows, need to loop and what better way than a cursor which uses the values from its select statement that get stored in memory. Off course, a batch insert is better performing like what Greg has given i.e. only 1 round trip to database and table lock held for lesser period unless autocommit is set to true i.e. without transaction.
0
Independent Software Vendors: We Want Your Opinion

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
cybersappAuthor Commented:
I don't quite understand the Cursor approach.  Can you guide me a little further with what I currently have in my procedure?
0
 
Chris LuttrellSenior Database ArchitectCommented:
Please don't send someone new down the "Use a Cursor" path.  He is in 2005 and can use a CTE or Table Variables to get what he needs and not use cursor.  I cannot dig into his code and descibe it in detail now (at work) but maybe Greg can look at what he is doing and apply a CTE to it, thats how I would approach trying to solve it.  Will follow up later if no one has done so.
0
 
cybersappAuthor Commented:
This is what I have fighting down the cursor path.  It works however the @N_ID doesn't grab the right  node ID.  It grabs the first one but nothing else.  Do I have to create a seperate cursor for that as well?  If so, how do I join the information when the cursor opens?  Lossed and perplexed :\

Thanks everyone for your help!  I greatly appreciate it.
if exists (select * from sysobjects where name = N'dbm_HostName2Trap2')
 
   drop procedure dbm_HostName2Trap2
go
 
CREATE PROCEDURE  dbm_HostName2Trap2
AS
SET IDENTITY_INSERT dbo.Traps ON
 
Declare @T_ID int
Declare @IP_Address varchar(15)
Declare @Hostname varchar(255)
Declare @N_ID int
-- Declare @iLoop int
-- Declare @iNextRowId int
-- Declare @iCurrentRowId int
DECLARE @IPCursor CURSOR 
 
SET @IPCursor = CURSOR FAST_FORWARD 
FOR 
Select Hostname,IP_Address
From IP_Resolve
 
SELECT @T_ID = (MAX(TRAPID) + 1) FROM Traps;
-- SELECT @Hostname = Hostname from IP_Resolve;
-- SELECT @IP_Address = IP_Address from IP_Resolve;
Select @N_ID = NodeID from Nodes,IP_Resolve where IP_Resolve.IP_Address = Nodes.IP_Address;
 
OPEN @IPCursor 
FETCH NEXT FROM @IPCursor 
INTO @Hostname,@IP_Address
 
WHILE @@FETCH_STATUS = 0 
BEGIN 
SELECT @T_ID = (MAX(TRAPID) + 1) FROM Traps;
Select @N_ID = NodeID from Nodes,IP_Resolve where IP_Resolve.IP_Address = Nodes.IP_Address;
 
INSERT INTO dbo.Traps (TrapID,EngineID,DateTime,IPAddress,Community,Tag,Acknowledged,Hostname,NodeID,TrapType,ColorCode) VALUES ( @T_ID,'2',DEFAULT,@IP_Address,'public',DEFAULT,DEFAULT,@Hostname,@N_ID,DEFAULT,DEFAULT ); 
 
FETCH NEXT FROM @IPCursor 
INTO @Hostname,@IP_Address
END 
 
CLOSE @IPCursor 
DEALLOCATE @IPCursor 
GO

Open in new window

0
 
SharathData EngineerCommented:

Why do you want to process row by row? Why can't you insert data directly? Is there any reason for that?
0
 
SharathData EngineerCommented:
Can you explain with some sample data?
0
 
cybersappAuthor Commented:
I don't have a sample available however I can explain.  The process here is for someone to update one table IP_Resolve.  Our Traps table purge itself every 7 days.  So, I've created a script to populate the data every 6 days so that the application can resolve Ip addresses from the IP_Resolve table.  The design is for someone who doesn't know anything about SQL to populate and manage a table as opposed to a script.
With that said, the Traps table require the entries listed above and the Ip_Resolve table only contains the ID,IP_Address, Hostname
The Nodes table holds the Node_ID that the application uses for other functions

I hope I've explained enough for you.
0
 
Chris LuttrellSenior Database ArchitectCommented:
Looking at all your examples, you can accomplish loading all the data from the IP_Resolve data into your Traps table with the following insert statement.  No Loops, No Cursors.  See if this is not what you need.
Some explanation on what it is doing:
This is letting the TrapID column get set by the Identity column.  If the table is cleared every 7 days and you want it to start back at 1, then
Truncate Table Traps
will clear out all data and reset the Identity column.
You do not need to specify all the columns that will get the DEFAULT values, that will just happen.
INSERT INTO dbo.Traps (EngineID,IPAddress,Community,Hostname,NodeID) 
SELECT '2',IP_Resolve.IP_Address,'public',IP_Resolve.Hostname,Nodes.NodeID
FROM IP_Resolve
INNER JOIN Nodes ON IP_Resolve.IP_Address = Nodes.IP_Address; 

Open in new window

0
 
cybersappAuthor Commented:
I will look at this again on Monday.  Thanks again for your help.
0
 
Mark WillsTopic AdvisorCommented:
Well, there are a few assumption about the structure of your TRAPS table.

Certainly if TrapID was an identity column will auto populate and auto increment the TrapID column and it also seems to be the main reason for looping around in a cursor, ie, to increment trapid.

So, that is probably the best solution. To recreate your TRAPS table with TrapID as an identity column. The you can update as per the very first posting and the posting above.

To do that, simply create your table like :

create table TRAPS (
   TrapID INT identity primary key clustered,                      -- added in the primary key stuff....
   engineID ......etc
   )

If an existing table, then can change it to an identity by going into table design and click on that column then down the bottom you need to expand the group for Identity and turn it on - cannot be null.

Then your insert statement, you simply ignore that column and it will autopopulate.



There are other ways if you do not want to make it an identity column. You can use a query to return a row_number() e.g.



insert into traps(trapid, engineid, ipaddress, community, hostname, nodeid)

select rn+max_trapid, engineid, ip_address,community, hostname, nodeid
from (
     select row_number() over (order by r.ip_address) as rn, (select isnull(max(trapid),0) from traps) as max_trapid,
               '2' as engineid, r.ip_address, 'public' as cummunity, r.hostname, n.nodeid
     from ip_resolve r
     inner join nodes n on r.id_address = n.ip_address ) as sq



Now, if you really want I can walk you through your cursor code as well. Generally speaking if there is a straight SQL query, it is going to outperform your cursor query, so sometimes, it might seem easier to get the cursor going, but a lot of the time, there is a straight query alternative - even if you want to create your own counter.

0
 
Mark WillsTopic AdvisorCommented:
Oh, by the way, the important thing in your query is to gen the join happing from IP_Resolve to Nodes and that is probably why you were haveing difficulty with getting all the information you thought you were meant to be getting.

Thought I would just add that cusros are not immediately a bad thing, it is just that they have fairly specific applications and a lot of time people first resort to a cursor rather than thinking about the query. And that is where it can be an "ordinary" answer rather than the "correct" solution.

But please, let us know if you want to walk through the cursor code - not in terms of endorsement, but to understand it...
0
 
Chris LuttrellSenior Database ArchitectCommented:
mark,
in the original code snippet it had "SET IDENTITY_INSERT dbo.Traps ON" so I believe the table already has the Identity column needed.
0
 
Mark WillsTopic AdvisorCommented:
Well spotted, I was really responding more so to the discussion.

If indeed it is an identity column, then there is absolutely no need for calculating a tableid, so the aproach indicated by hjestersgfrind was pretty much spot on, pity it missed the join to nodes, which of course CGLuttrell did pick up on.
0
 
Mark WillsTopic AdvisorCommented:
Might be worth having a look at the structure of that TRAPS table...
0
 
cybersappAuthor Commented:
Mark_Wills: Yeah the Traps table is Identity is set to ON. So I think we are straight in that regard.  I had a basic query but changed it to a cursor from recommendations from the forum.  I'm new to writing SQL so I need a little more assistance than most people, but I'm a quick learner.  if you could walk me through what I have to make the procedure work or assist me with CGLutrell suggestion that would be great!
0
 
Mark WillsTopic AdvisorCommented:
Sure,

First up let us look at the different pieces of the puzzle...

1) Traps is being populated.
2) Traps has an identity column TrapID.
3) When we insert, we want to make sure TrapID is incremented
4) We need to get information mainly from IP_Resolve, but also from Nodes

OK, so lets look at the overall task first - denoted by point 1.

Somewhere we need to construct an INSERT TRAPS statement. Which is terrific, but where and how do we get to the data ? That is always the 64 million dollar question that consumes most T-SQL programmers time, and is where we start the journey.

We know we have to get the data from IP_Resolve and Nodes. One way we can connect those two tables is using a join, or we could even lookup if there is nothing obvious to join on. So that is our first task and we can start with a select statement...

Select R.*, N.*
from IP_Resolve R
Inner join Nodes N on r.id_address = n.ip_address


Notice how I use a table alias for both IP_Resolve and Nodes (being R and N respectively). Using an Alias means that the table is now referenceable using those aliases, and save a bit of typing, creates flexibility (could change the table without having to change the rest of the query), and allows for duplicate sources (could have another table of the same name, but with a different alias).

Whe I run that query, I am looking at the number of rows and the columns being selected to ensure I am getting the rught results. So let us fine tune that query a bit more, and include some of the other things we want.

Select '2' as engineid, r.ip_address, 'public' as cummunity, r.hostname, n.nodeid
from ip_resolve r
inner join nodes n on r.id_address = n.ip_address
order by r.ip_address,n.nodeid  

Now, I can keep playing with that query until I am happy with the results, and maybe because there are other "common" columns, might need to fine tune that join a bit more, or even add in a "where" clause to help filter those results. For example, there might be duplicates, and we just want each distinct combination of IP_Address and Nodeid per Hostname, so we can add that in. Also, we may as well exclude the (not real, but as an example) "test" server (so you see a where clause in action).

Select Distinct '2' as engineid, r.ip_address, 'public' as cummunity, r.hostname, n.nodeid
from ip_resolve r
inner join nodes n on r.ip_address = n.ip_address
where r.hostname <> 'My_Test_Dummy_Dev_QA_Server'
order by r.ip_address,n.nodeid  

By now our query should be complete. In so much as we have identified where we are getting the data from and happy with the results. So, now we can start looking at the INSERT statement.

It is at this point in time when you can decide what kind of action needs to be taken. It could be that a cursor is appropriate, however, it normally is not. Looking at row by row is what the cursor is all about, and allowing SQL to get back a complete "set" of data so it can manage it is much much more efficient.

First up, always use the column list. If your table changes underneath, then using the column list will keep it one step independant of some of those changes. It is best to always code for the known conditions. We also have our data source from the query above, so now our insert statement will take on the form of INSERT <table> (<column_list>) select <column_list> from <datasource>. Next thing to look at is the inserted <column_list>...

We need / must populate a few 'known' columns. They are any column that cannot be NULL, along with the data elements we want to know. There are also a few columns we do not have to worry about. They are those columns which can be null, or, SQL is going to manage for us such as columns with a default value, or IDENTITY columns. With that in mind, let us now build our insert statement....

INSERT TRAPS (engineid, ipaddress, community, hostname, nodeid)     -- note trapid is an identity so SQL will do that for us.
SELECT Distinct '2' as engineid, r.ip_address, 'public' as cummunity, r.hostname, n.nodeid
FROM ip_resolve r
INNER JOIN nodes n on r.ip_address = n.ip_address                            -- inner join means that a row MUST exist for r.ip_address in the NODES table
where r.hostname <> 'My_Test_Dummy_Dev_QA_Server'
order by r.ip_address,n.nodeid                                                             -- no real need for an order by in this case, it was more so we can see the rows in our query


And that is basically what we needed to achieve. Looking now at CGLuttrell's posting :

INSERT INTO dbo.Traps (EngineID,IPAddress,Community,Hostname,NodeID)
SELECT '2',IP_Resolve.IP_Address,'public',IP_Resolve.Hostname,Nodes.NodeID
FROM IP_Resolve
INNER JOIN Nodes ON IP_Resolve.IP_Address = Nodes.IP_Address;

It has all the same elements as we have built up, and so, the individual components should hopefully be recognisable to you, and if not please let me know which parts are still a bit confusing...

Now all we have to do is wrap that up in a procedure if we still want to. Normally a procedure is good for coding when there are various conditional steps, or parameters involved. In this case you can execute the query directly, so doesn't really have to be encapsulate in a stored procedure, but no real reason why it can't be. If it is a stored procedure the one big advantage is that you are effectively hiding the raw code from the end-user space and can be desirable to help prevent SQL injection (where additional SQL commands can be inserted).

There is one other facet to consider. And that is the possibility of duplicates in terms of data content. That can sometimes happen when there is a surrogate key such as an identity used for the primary key. It means I could add in the same ip_address a few times, and it will never be regarded as a duplicate. There are a couple of strategies for that, and believe the best is to explicity code and cater for that. Found one of the easiest ways is to include an EXISTS statement which simply tests a results set. So the complete query statement could be more like :


INSERT TRAPS (engineid, ipaddress, community, hostname, nodeid)     -- note trapid is an identity so SQL will do that for us.
SELECT Distinct '2' as engineid, r.ip_address, 'public' as cummunity, r.hostname, n.nodeid
FROM ip_resolve r
INNER JOIN nodes n on r.ip_address = n.ip_address                            -- inner join means that a row MUST exist for r.ip_address in the NODES table
WHERE r.hostname <> 'My_Test_Dummy_Dev_QA_Server'
AND NOT EXISTS (Select NULL from traps T where t.engineid = '2' and t.ip_address = r.ip_address and t.hostname = r.hostname and t.nodeid = n.nodeid)


The above addition of NOT EXISTS is now testing to see if there are existing rows with the same columns, and we want to add if not already there.

One comment, we populated engineid with '2' and if it is an integer then do not use the single quotes, e.g. 2 as engineid

Do you want the original stored procedure exlained now ?



0
 
Mark WillsTopic AdvisorCommented:
sorry, in the above I have inadvertantly used r.id_address and should be r.ip_address....
0
 
cybersappAuthor Commented:
Yes please do explain the stored procedure.  I have a general understanding, but you may be able to clear it up for me.  Thanks for taking the time too!
0
 
Mark WillsTopic AdvisorCommented:
No Problems...

Might be best if I add comments to the stored procedure itself. But first a general overview...

Using cursors normally takes the form of row by row operations where the only possible solution is you must do something with individual rows to get the answer.

Cusrosrs normally take on the form :

DECLARE my_cursor CURSOR FAST_FORWARD READ_ONLY FOR SELECT <column_list> from <datasource>                -- note fast_forward also makes it read_only, so is redundant.

OPEN my_cursor

FETCH NEXT FROM my_cursor INTO @variables
WHILE @@FETCH_STATUS = 0
BEGIN
      <do stuff row by row>
      FETCH NEXT FROM my_cursor INTO @variables
END

CLOSE my_cursor
DEALLOCATE my_cursor


Your stored procedure is using a cursor variable and is not really needed. There is only one or two very specific instances where a cursor variable might be needed and that is in association with pretty tricky SQL to overcome dynamic SQL instances so is not part of this discussion.

Import bits here are using variables to store the specific row values, then using those variables within the loop (rather than the @N_ID select), and allowing SQL to do the right thing with default values.

Again, look at the comments, immediately above each statement and let me know if any further explanation is required.





-- next statement checks the existance of the stored procedure and drops it if already in existance
-- all objects are known to the database engine, so pretty much, you can always check to see if something exists.
 
if exists (select * from sysobjects where name = N'dbm_HostName2Trap2')
   drop procedure dbm_HostName2Trap2
go
 
 
-- now we are creating a new store procedure. If it is already there, could also use the ALTER verb in place of CREATE.
 
CREATE PROCEDURE  dbm_HostName2Trap2
AS
 
 
-- the next statement is telling the database engine that is it OK to manually populate the identity column. 
-- This is pretty dangerous, and only really used if an identity value needs to be preserved because it is is referred
-- to from some other data source. Generally do not do this.
SET IDENTITY_INSERT dbo.Traps ON
 
 
-- now creating a few variables to store values 
Declare @T_ID int
Declare @IP_Address varchar(15)
Declare @Hostname varchar(255)
Declare @N_ID int
 
 
-- this is an interesting one. You can declare a cursor variable, but is not needed in the vast majority of cases. 
-- Can also do :  DECLARE my_cursor CURSOR FAST_FORWARD FOR SELECT <column_list> from <datasource>
DECLARE @IPCursor CURSOR 
 
 
-- pointing the cursor to a data source - there is that "query" aspect again. The same 64 million dollar question
-- that we had to answer above. Main difference here is that the query in the previous posting would not be able
-- to answer the question for you, so we MUST resort to row by row processing.
SET @IPCursor = CURSOR FAST_FORWARD FOR Select Hostname,IP_Address From IP_Resolve
 
 
-- here we are setting up the variable @T_ID to be the currently 'known' maximum value of trapid from the TRAPS table
-- the only reason we are doing this is so we can increment the trapid value ourselves, and is not needed when we have
-- trapid as an identity column...
-- also, when doing this type of thing, need to take into account the possibility of a NULL value.
-- so it probably should be : SELECT @T_ID = ISNULL(MAX(TRAPID),0) + 1 FROM Traps
SELECT @T_ID = (MAX(TRAPID) + 1) FROM Traps;
 
 
-- here we are trying to set up the variable @N_ID being the nodeid for the IP_Address. 
-- big trouble here....
-- it is essentially not giving you each nodeid by IP_Resolve. It is just geeting the last join 
-- you are using an "equi-join", and much better to manage the join more explicitly as shown previously
-- this should really be inside the cursor loop. See below...
Select @N_ID = NodeID from Nodes,IP_Resolve where IP_Resolve.IP_Address = Nodes.IP_Address;
 
 
-- now we are opening the cursor and returning the table values (using fetch) into the variables 
-- @hostname and @ip_address, which need to match the select statement in the cursor definition
OPEN @IPCursor 
FETCH NEXT FROM @IPCursor INTO @Hostname,@IP_Address
 
-- @@fetch_status is automatically generated as a result of using the fetch statement. 0 means it returned a result OK
-- so then we encapsulate a series of statements within a BEGIN and END block.
WHILE @@FETCH_STATUS = 0 
BEGIN 
 
 
-- Having set up a starting point for @T_ID, all we should be doing is incrementing the variable
-- meaning : SET @T_ID = @T_ID + 1 
-- rather than having to do another read from the database.
SELECT @T_ID = (MAX(TRAPID) + 1) FROM Traps;
 
 
-- nodeid is specific to this IP_Resolve, so, what we need to be doing is using the ROW information ie @IP_Address
-- so this should be Select @N_ID = NodeID from Nodes where IP_Address = @IP_Address
Select @N_ID = NodeID from Nodes,IP_Resolve where IP_Resolve.IP_Address = Nodes.IP_Address;
 
-- now the insert statement 
-- no need to use the DEFAULT keyword. Like the identity column, they will get populated automatically
-- and also try hard not to use reserved words as column names
INSERT INTO dbo.Traps (TrapID,EngineID,DateTime,IPAddress,Community,Tag,Acknowledged,Hostname,NodeID,TrapType,ColorCode) VALUES ( @T_ID,'2',DEFAULT,@IP_Address,'public',DEFAULT,DEFAULT,@Hostname,@N_ID,DEFAULT,DEFAULT ); 
 
-- returning the next row  
FETCH NEXT FROM @IPCursor INTO @Hostname,@IP_Address
 
-- and the loop goes back to the above "WHILE" statement
END 
 
-- close the cursor and release the memory spaces / resources consumed by the cursor.
CLOSE @IPCursor 
DEALLOCATE @IPCursor 
GO

Open in new window

0
 
Mark WillsTopic AdvisorCommented:
Line 35 in the code snippet - let me explain that. When a single query cannot be used to resolve that 64 million dollar question, that is when you might need to resort to row-by-row operations and therefore need a cursor. I was not saying that you would have to use a cursor in this case, more so, when a cursor might be needed.
0
 
cybersappAuthor Commented:
Thanks!!!  I will plug into the matrix in about an hour to complete this procedure.  Thanks for your help and explanation.  I understand it better now.
0
 
Mark WillsTopic AdvisorCommented:
Happy to help...
0

Featured Post

Fill in the form and get your FREE NFR key NOW!

Veeam is happy to provide a FREE NFR server license to certified engineers, trainers, and bloggers.  It allows for the non‑production use of Veeam Agent for Microsoft Windows. This license is valid for five workstations and two servers.

  • 9
  • 7
  • 3
  • +3
Tackle projects and never again get stuck behind a technical roadblock.
Join Now