Stored Procedure - Close Cursor Error 2

[gertNo1]

Hi Guys, I'm new to Stored Procedures and cannot for the life of me see the error in this procedure. Here are the error msgs I'm getting:

Msg 102, Level 15, State 1, Procedure Get Unmatched SYNC vs DMS, Line 37
Incorrect syntax near ','.
Msg 102, Level 15, State 1, Procedure Get Unmatched SYNC vs DMS, Line 49
Incorrect syntax near 'MyCursor'.



Any Help would be appreciated.
Thanx so much

CREATE PROCEDURE [Get Unmatched SYNC vs DMS] AS
BEGIN
/* Pull recs that are on DMS but not on SYNC table */

/* Define every field you want to output */
DECLARE @Lname varchar(35)
DECLARE @Fname varchar(25)
DECLARE @Address varchar(32)
DECLARE @Address2 varchar(32)
DECLARE @City varchar(20)
DECLARE @State varchar(2)
DECLARE @Zip varchar(9)
DECLARE @Prod varchar(10)
DECLARE @EffDate varchar(8)
DECLARE @Last4 varchar(6)
DECLARE @LoanNum varchar(20)
/* Define Cursor */
DECLARE MyCursor cursor

For SELECT dbo.Matched_DMS.Lname, dbo.Matched_DMS.Fname, dbo.Matched_DMS.Addr1, 
	dbo.Matched_DMS.Last4, dbo.Matched_DMS.Loan_Num, dbo.Matched_DMS.Addr2, 
	dbo.Matched_DMS.city, dbo.Matched_DMS.state, dbo.Matched_DMS.zip, 
	dbo.Matched_DMS.Prod, dbo.Matched_DMS.effDate
FROM dbo.Matched_DMS 

Open MyCursor;		/*Works like an ARRAY*//*holds current record	*/
FETCH NEXT FROM MyCursor INTO @LoanNum, @Lname, @Fname, @Address, @Address2, 
							  @City, @Last4, @State, @zip, @prod, @effDate 

/*When @@Fetch_Status = 1 EOF has been reached	*/ 
WHILE @@Fetch_Status = 0
	BEGIN
	     INSERT INTO Unmatched_DMS_Recs /*Output tbl; fields in output tbl (Below)	*/ 
		 SELECT (Loan_Num, L4, Lname, Fname, addr, addr2, city, [state], zip, prod, effdate)
		 WHERE WF.SYNC.Addr1 <> @Address AND
			   WF.SYNC.Lname <> @Lname AND 
			   WF.SYNC.Fname <> @Fname AND
			   WF.SYNC.LAST4 <> @Last4 

				/*Continue FETCH Command until EOF	*/		
	FETCH NEXT FROM MyCursor INTO @LoanNum, @Lname, @Fname, @Address, @Address2, 
							  @City, @Last4, @State, @zip, @prod, @effDate 
	
	END
CLOSE MyCursor
DEALLOCATE MyCursor

 

[gmmastros]

Remove the parenthesis from this line:

SELECT [!]([/!]Loan_Num, L4, Lname, Fname, addr, addr2, city, [state], zip, prod, effdate[!])[/!]

For the second error....

With stored procedures, you do not need to have 'begin' at the beginning, but you can. Your choice. If you use Begin, then you must use End. So, put another line at the end of your code with 'End' on that line.

[COLOR=blue]CREATE[/color] [COLOR=blue]PROCEDURE[/color] [[COLOR=blue]Get[/color] Unmatched SYNC vs DMS] [COLOR=blue]AS[/color]
[COLOR=blue]BEGIN[/color]
[COLOR=green]/* Pull recs that are on DMS but not on SYNC table */[/color]

[COLOR=green]/* Define every field you want to output */[/color]
[COLOR=blue]DECLARE[/color] @Lname [COLOR=blue]varchar[/color](35)
[COLOR=blue]DECLARE[/color] @Fname [COLOR=blue]varchar[/color](25)
[COLOR=blue]DECLARE[/color] @Address [COLOR=blue]varchar[/color](32)
[COLOR=blue]DECLARE[/color] @Address2 [COLOR=blue]varchar[/color](32)
[COLOR=blue]DECLARE[/color] @City [COLOR=blue]varchar[/color](20)
[COLOR=blue]DECLARE[/color] @State [COLOR=blue]varchar[/color](2)
[COLOR=blue]DECLARE[/color] @Zip [COLOR=blue]varchar[/color](9)
[COLOR=blue]DECLARE[/color] @Prod [COLOR=blue]varchar[/color](10)
[COLOR=blue]DECLARE[/color] @EffDate [COLOR=blue]varchar[/color](8)
[COLOR=blue]DECLARE[/color] @Last4 [COLOR=blue]varchar[/color](6)
[COLOR=blue]DECLARE[/color] @LoanNum [COLOR=blue]varchar[/color](20)
[COLOR=green]/* Define Cursor */[/color]
[COLOR=blue]DECLARE[/color] MyCursor [COLOR=blue]cursor[/color]

[COLOR=blue]For[/color] [COLOR=blue]SELECT[/color] dbo.Matched_DMS.Lname, dbo.Matched_DMS.Fname, dbo.Matched_DMS.Addr1,
    dbo.Matched_DMS.Last4, dbo.Matched_DMS.Loan_Num, dbo.Matched_DMS.Addr2,
    dbo.Matched_DMS.city, dbo.Matched_DMS.state, dbo.Matched_DMS.zip,
    dbo.Matched_DMS.Prod, dbo.Matched_DMS.effDate
[COLOR=blue]FROM[/color] dbo.Matched_DMS

[COLOR=#FF00FF]Open[/color] MyCursor;        [COLOR=green]/*Works like an ARRAY*/[/color][COLOR=green]/*holds current record    */[/color]
[COLOR=blue]FETCH[/color] [COLOR=blue]NEXT[/color] [COLOR=blue]FROM[/color] MyCursor [COLOR=blue]INTO[/color] @LoanNum, @Lname, @Fname, @Address, @Address2,
                              @City, @Last4, @State, @zip, @prod, @effDate

[COLOR=green]/*When @@Fetch_Status = 1 EOF has been reached    */[/color]
[COLOR=blue]WHILE[/color] @@Fetch_Status = 0
    [COLOR=blue]BEGIN[/color]
         [COLOR=blue]INSERT[/color] [COLOR=blue]INTO[/color] Unmatched_DMS_Recs [COLOR=green]/*Output tbl; fields in output tbl (Below)    */[/color]
         [!]SELECT Loan_Num, L4, Lname, Fname, addr, addr2, city, [state], zip, prod, effdate[/!]
         [COLOR=blue]WHERE[/color] WF.SYNC.Addr1 <> @Address AND
               WF.SYNC.Lname <> @Lname AND
               WF.SYNC.Fname <> @Fname AND
               WF.SYNC.LAST4 <> @Last4

                [COLOR=green]/*Continue FETCH Command until EOF    */[/color]        
    [COLOR=blue]FETCH[/color] [COLOR=blue]NEXT[/color] [COLOR=blue]FROM[/color] MyCursor [COLOR=blue]INTO[/color] @LoanNum, @Lname, @Fname, @Address, @Address2,
                              @City, @Last4, @State, @zip, @prod, @effDate
    
    [COLOR=blue]END[/color]
[COLOR=blue]CLOSE[/color] MyCursor
[COLOR=blue]DEALLOCATE[/color] MyCursor
[!]End[/!]

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[gmmastros]

Ahh... 1 more comment.

You do not need to use a cursor for this code. If you continue using a cursor, this stored procedure will take longer to execute than it needs to.

You can write a 'set based' query to do the same thing the cursor is doing. If you're interested in doing this, let me know and I'll help you convert this to a set based insert statement instead of a cursor. Your database will thank you.

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[SQLSister]

I second George, you should almost never use a cursor and especially not for inserts, selects, updates or deletes. Cursors will take many many times more time to process than set-based queries. I have seen cursors that take hours go down to minutes or even seconds when converted to a set-based query. It is a very bad habit to write cursors even when the data set is small enough that they don't have a big performance hit becasue then you will not learn how to write set-based queries and will not use them when needed. Plus over time all those just a little bit slower things add up to poor performance and in some ways are harder to find and fix than the big performance hits just becasue you have to fix so much stuff not just one poorly performing stroed proc.

"NOTHING is more important in a database than integrity." ESquared

 

[gertNo1]

You Guys are the very B-E-S-T. I can't wait to see how I can optimize this query.

Thank you sooooo much George and SQLSister.

I have a question if I only have one BEGIN why do I need to END statments?

Be well,

 

[gmmastros]

>> I have a question if I only have one BEGIN why do I need to END statments?

You need to have the same number of each. Originally, you had 2 begins and only 1 end.

[COLOR=blue]CREATE[/color] [COLOR=blue]PROCEDURE[/color] [[COLOR=blue]Get[/color] Unmatched SYNC vs DMS] [COLOR=blue]AS[/color]
[COLOR=blue]BEGIN[/color]
[COLOR=blue]WHILE[/color] @@Fetch_Status = 0
    [COLOR=blue]BEGIN[/color]
    [COLOR=blue]END[/color]

[COLOR=blue]End[/color] [COLOR=green]-- This one was missing[/color]

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[gmmastros]

Before I give you advice on removing the cursor, can you please explain, in words, what this query is trying to do. It appears to be inserting records from one table in to another based on certain conditions. What are the table names, and the conditions?



-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[gertNo1]

I have an in-house table and a table supplied by a client. I am trying to insert records that are on our in-house table that do not appear on the client supplied table.

Since I am new I'm not sure I am even going about this correctly. If you have a better way please let me know. I am always open to new input and good ideas.

client supplied table: WF.SYNC
in-house table: dbo.Matched_DMS
output table: Unmatched_DMS_Recs

 

[gertNo1]

OBTW I got my DUH! moment when I looked at the procedure again and saw the BEGIN at the start of the procedure. Ahh! Tunnel vision will get you every time.

Thanx so much,

 

[gmmastros]

Temporarily forget about inserting in to another table... Does this query return the correct rows?

Select M.*
From   Matched_DMS As M
       Left Join WF.Sync As S
         On  M.Addr1 = S.Addr1
         And M.Lname = S.Lname
         And M.Fname = S.Fname
         And M.Last4 = S.Last4
Where  S.LName Is NULL

This 'should' return rows from the Matched_DMS table that do not exist in the WF.Sync table.




-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[SQLSister]

Try this:

  INSERT INTO Unmatched_DMS_Recs (Loan_Num, L4, Lname, Fname, addr, addr2, city, [state], zip, prod, effdate)
         SELECT Loan_Num, L4, Lname, Fname, addr, addr2, city, [state], zip, prod, effdate
		FROM dbo.Matched_DMS M 
		left Join  WF.SYNC W
               on W.Addr1 = M.Addr1 AND
               W.Lname = M.Lname AND
               W.Fname = M.Fname AND
               W.LAST4 = MLast4
		where W.Lname is null

of course try the select first to make sure you are getting the records you want.


"NOTHING is more important in a database than integrity." ESquared

 

[gertNo1]

Hi Guys, thanks so much for the feedback on performing this query without a cursor. I'll try your suggestions.

To answer George's question no it does not return the correct rows. I am trying to do a outer join on multiple fields but it doesn't seem to be working. When I tried it in Access I got the obligatory “Ambiguous joins” problem. Someone said it would work better in SQL Server.

Thanx again

 

[gertNo1]

Hi Guys, that's great for Lname but what about the other fields?

Someone suggested doing a subquery, first eliminate records where Lname = Null, then Fname = Null, address etc. Or is there another way?

Thanx much,

 

[gmmastros]

Can you post some sample data from your tables. (replacing actual names with made up ones). Then, identify the data that is NOT right. It will make it easier for us to help. I would NOT use a subquery here. It won't perform as well.


-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[gertNo1]

No problem to post the data. But I guess my question is what about the records where the Fname and Lname are equal and the adddr1 is not equal? How do we identify those records?

Since we are trying to locate all unmatched records based on all of the fields we need to test/match on all of them. I realize some of the results will overlap but some won't.

I probably should have done a better job of explaining what the concept of the project was. Sometimes I really miss the mark.

Thanx

 

[ESquared]

Why are you trying SQL Server queries in Access?

 

[gertNo1]

Hi ESquared, thanks for joining the fray. I first wrote the query in Access and someone stated that it would be easier to do in SQL Server. So far it looks exactly as it did in Access. I still cannot create an UNmatched scenario on more than one field.

I guess I will have to create a series of subqueries to get anywhere near the desired results.

Thanx so very much to everyone who responded.

 

[ESquared]

We can help you if you will only tell us enough about what you are trying to do.