April 28, 2016 at 8:39 pm
So I just learned about using Cursors in my SP to loop through a dataset. So I set up a dummy test. So I have 3 columes (Autex, OrigCap, LastDate). Autex is basically the acct number. In this table, I can have accts with 2 rows and accts with 2+ rows of data for OrigCap and LastDate.
So my first highlighted SELECT (in Green), grabs the dataset and the OrigCap is Null will only bring back one instance of each account because each account can only have 1 null. I have tested this and it does give me the right dataset where I only get 1 instance of each Autex where OrigCap is NULL.
All I want to do is run a COUNT and see how many times Autex is actually in this table. Not just Null but how many rows exist for this Autex in the whole table.
If the result is 2, then update the one NULL value with the number (1). If it exist in the table >2, then update that one NULL value with the number (2).
Question: Do I have the Set @AutexCount in the right place to test each Autex as it's looping? ( In Yellow)
Is there something wrong with my IF Statement? Seems like a pretty straight forward thing and just update the one NULL per Autex with either a 1 or a 2.
So when I run this, it just runs and runs and runs but nothing is happening. I'm showing 0 rows affected and it just keeps running till I have to stop it manually. it's like it's stuck in a loop or something with no instructions on what to do next.
Please help a newbie trying to learn.
Thanks!
USE [CroftDB]
GO
/****** Object: StoredProcedure [dbo].[usp_ContWd_DEL] Script Date: 4/28/2016 9:38:25 PM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
-- Cursors is a way to LOOP through a dataset and perform a task on each record
-- ******************************************************
-- Learned to user CURSORs from this site
-- https://www.youtube.com/watch?v=RHRjLd0bEaQ
-- ******************************************************
ALTER PROCEDURE [dbo].[usp_ContWd_DEL]
-- Add the parameters for the stored procedure here
AS
-- Parameters for CURSORs has to start with Declare after the 'AS' while other normal SP parameters go before the 'AS' with commas
Declare@Autex as varchar(8)
Declare@LastDate as date
Declare @AutexCount as float
-- You declare your Cursor name
Declare ContWdCursor CURSOR
-- You have to have a FOR for Cursors, don't know why
FOR
-- The select statement to get the dataset you want to test on
[highlight="#2FB315"](SELECT Autex, Date as LastDate FROM ClientsContWd WHERE (OrigCap IS NULL) and Autex <> '8811sub' and Autex <> '8813sub')[/highlight]
-- Open the Cursor
OPEN ContWdCursor
-- Fetch means you are putting each rows data into the parameters so you can use it. NOTE how it's in the same order as the SELECT statement
FETCH NEXT FROM ContWdCursor INTO @Autex, @LastDate
-- This while is a must for Cursors. The Fetch statement just means keep getting the next row from the above dataset until no more
WHILE @@FETCH_STATUS = 0
[highlight="#F8FF15"]Set @AutexCount = (SELECT COUNT(Autex) AS Expr1 FROM ClientsContWd WHERE (Autex = @Autex))[/highlight]
-- This is where you will do something with each row
BEGIN
If @AutexCount = 2
Begin
UPDATE ClientsContWd SET OrigCap = 1 WHERE (Autex = @Autex) AND (Date = @LastDate)
Return
End
Else If @AutexCount > 2
Begin
UPDATE ClientsContWd SET OrigCap = 2 WHERE (Autex = @Autex) AND (Date = @LastDate)
Return
End
--This means to get the next row and do this task again.
FETCH NEXT FROM ContWdCursor INTO @Autex, @LastDate
END
-- Must have these two
CLOSE ContWdCursor
DEALLOCATE ContWdCursor
April 29, 2016 at 4:41 am
I'm not sure why you want to achieve this with a Cursor to be honest, you could do this simply in a dataset update.
With some made up data, would this achieve what your aiming for?
--Create sample data table, I'm sure your data has a lot more columns in than this!
Create table ClientsContWd (Autex varchar(10),
LastDate Date,
OrigCap int)
Go
--Create some test data
Insert into ClientsContWd_test (Autex, LastDate)
Values ('AA111', '01-Jan-2015'),
('AA111', '05-Jan-2015'),
('AA111', '05-Jan-2015'),
('AA111', '12-Jan-2015'),
('AA175', '12-Jan-2015'),
('AA175', '12-Jan-2015'),
('AA175', '12-Jan-2015'),
('AA175', '25-Jan-2015'),
('AA175', '25-Jan-2015')
Go
--begin transaction
--have a look at what the data looks like now
Select * from ClientsContWd_test
--Update the table using a subquery and count. I don't know if it's applicable, but this would return NULL if the Autex/Last Date count is 1.
Update ClientsContWd_test
Set OrigCap = (Select case when count(C.Autex) = 2 then 1
when count(C.Autex) > 2 then 2 end
from ClientsContWd_test C
where C.Autex = ClientsContWd.Autex
and C.LastDate = ClientsContWd.LastDate)
Go
--Get the new view
Select * from ClientsContWd_test
--Rollback
This results in:
A111 on 01 Jan having a NULL OrigCap
A111 on 05 Jan having a value of 1 in OrigCap
A111 on 12 Jan having a NULL OrigCap
AA175 on 12 Jan having a value of 2 in OrigCap
AA175 on 25 Jan having a value of 1 in OrigCap
Hope that helps if it's what you're aiming for 🙂
Thom~
Excuse my typos and sometimes awful grammar. My fingers work faster than my brain does.
Larnu.uk
April 29, 2016 at 7:33 am
To start on why it stays in an infinite loop, you have the Set @AutexCount before the begin, so this is the only statement running as a loop without any way to exit the loop.
Second, this way of coding SQL is commonly referred as RBAR (Row-By-Agonizing-Row). It comes from procedural programming, but it's a bad idea in SQL. To write SQL you need to think of each set as a whole, instead of thinking on rows. The first step is to stop thinking what you want to do with a row and start thinking what you want to do with a column.
Thom posted an example on how you could rewrite this code with less lines and in a more efficient way. My only concern is that you're not really using the date column for calculation, so some modification might need to be done.
I'm not sure about the logic, but this is my option. Be sure to understand what's happening and ask any questions that you might have.
UPDATE cw
OrigCap = CASE WHEN CountAutex = 2 THEN 1
WHEN CountAutex > 2 THEN 2
END
FROM ClientsContWd ccw
CROSS APPLY (SELECT COUNT(*) AS CountAutex
FROM ClientsContWd i
WHERE i.Autex =ccw.Autex
HAVING COUNT(*) >= 2) ccc
WHERE OrigCap IS NULL
and Autex <> '8811sub'
and Autex <> '8813sub';
April 29, 2016 at 8:59 am
Thanks for the response guys. I tried to be as clear as possible but I don't think I did a good enough job. I understand how to update the whole column as you guys have mentioned. The reason, I'm trying to learn the row by row is the final product will not be injecting 1 or 2 in the OrigCap field. Based on if it sees 1 or 2, it will run a whole bunch of other statements + doing calculations all based on different parameters and then get a real dollar value that will actually go into the OrigCap field. The @LastDate plays into the final calculations. All of it can not be done by just a sub query and that's why I have to do it row by row. Sorry about the confusion.
My thinking was that If I can get it to do just a simple inject 1 or 2 for now, I can fill in all the other codes later.
Luis, I tried putting the Set @AutexCount after the Begin and it didn't get stuck in the loop. It actually did nothing. It just ran and said completed with no updates of 1s or 2s.
OPEN ContWdCursor
-- Fetch means you are putting each rows data into the parameters so you can use it. NOTE how it's in the same order as the SELECT statement
FETCH NEXT FROM ContWdCursor INTO @Autex, @LastDate
-- This while is a must for Cursors. The Fetch statement just means keep getting the next row from the above dataset until no more
WHILE @@FETCH_STATUS = 0
-- This is where you will do something with each row
[highlight="#AEFFFF"]BEGIN
Set @AutexCount = (SELECT COUNT(Autex) AS Expr1 FROM ClientsContWd WHERE (Autex = @Autex))[/highlight]
If @AutexCount = 2
Begin
UPDATE ClientsContWd SET OrigCap = 1 WHERE (Autex = @Autex) AND (Date = @LastDate)
Return
End
Else If @AutexCount > 2
April 29, 2016 at 9:33 am
As part of personal experience, I would suggest to get away from cursors for this thing. Making complex calculations one row at a time will slow down your system in an awful way.
We can help you getting a better way to handle everything in sets.
For now, try to print @AutexCount value to see if you're actually having anything to update. It might always have a 1 as a value.
Viewing 5 posts - 1 through 4 (of 4 total)
You must be logged in to reply to this topic. Login to reply