loop problem

Jassim Rahma

I am using the following ocde to loop through existign records and create account summart record for every client but it seems looping with no end and it's creating an endless account summary records for the first client only.. what's wrong?

USE takhlees

SET ANSI_NULLS ON
SET QUOTED_IDENTIFIER ON
SET NOCOUNT ON

DELETE FROM accounts_summary

-- reset primary key

INSERT INTO accounts_summary (account_number, account_name, core_account, is_read_only) VALUES (10000000001, 'CASH', 'True', 'True')
INSERT INTO accounts_summary (account_number, account_name, core_account, is_read_only) VALUES (10000000002, 'ACCOUNTS RECEIVABLE', 'True', 'True')

DECLARE @client_id int, @client_cpr varchar(50), @client_name varchar(255), @account_number_string VARCHAR(50), @account_number numeric(38,0)

DECLARE account_numbers CURSOR FOR
SELECT clients.client_id, clients.client_cpr, clients.client_name
FROM clients
ORDER BY clients.client_name

OPEN account_numbers

FETCH NEXT FROM account_numbers INTO @client_id, @client_cpr, @client_name

WHILE @@FETCH_STATUS = 0
BEGIN
INSERT INTO accounts_summary (client_category, client_id, cr_cpr_number, account_number, account_name) VALUES ('I', @client_id, @client_cpr, SCOPE_IDENTITY(), @client_name)
-- get random from 1 to 7
SET @account_number_string = RIGHT('0' + convert(varchar(1), DATEPART(WEEKDAY, GETDATE())), 1)
-- get random from 1 to 365
SET @account_number_string = @account_number_string + RIGHT('000' + convert(varchar(3), DATEPART(DAYOFYEAR, GETDATE())), 3)
SET @account_number_string = @account_number_string + RIGHT('0000000' + convert(varchar(7), SCOPE_IDENTITY()), 7)
SET @account_number = convert(bigint, @account_number_string)
UPDATE accounts_summary SET account_number = @account_number WHERE account_id = SCOPE_IDENTITY()
UPDATE clients SET account_number = @account_number WHERE client_id = @client_id

FETCH NEXT FROM account\_numbers INTO @client\_id, @client\_cpr, @client\_name

END

CLOSE account_numbers
DEALLOCATE account_numbers

Jassim Rahma

does it have anything to do with this:

WHILE @@FETCH_STATUS = 0

Jassim Rahma

this is the modeified code ro make the SCOPE_IDENTITY clear but of course still the same problem

DECLARE @rec_id int, @client_id int, @client_cpr varchar(50), @client_name varchar(255), @account_number_string VARCHAR(50), @account_number numeric(38,0)

DECLARE account_numbers CURSOR FOR
SELECT clients.client_id, clients.client_cpr, clients.client_name
FROM clients
ORDER BY clients.client_name

OPEN account_numbers

FETCH NEXT FROM account_numbers INTO @client_id, @client_cpr, @client_name

WHILE @@FETCH_STATUS = 0
BEGIN
INSERT INTO accounts_summary (client_category, client_id, cr_cpr_number, account_number, account_name) VALUES ('I', @client_id, @client_cpr, 0, @client_name)
SET @rec_id = SCOPE_IDENTITY()
-- get random from 1 to 7
SET @account_number_string = RIGHT('0' + convert(varchar(1), DATEPART(WEEKDAY, GETDATE())), 1)
-- get random from 1 to 365
SET @account_number_string = @account_number_string + RIGHT('000' + convert(varchar(3), DATEPART(DAYOFYEAR, GETDATE())), 3)
SET @account_number_string = @account_number_string + RIGHT('0000000' + convert(varchar(7), @rec_id), 7)
SET @account_number = convert(bigint, @account_number_string)
UPDATE accounts_summary SET account_number = @account_number WHERE account_id = @rec_id
UPDATE clients SET account_number = @account_number WHERE client_id = @client_id

FETCH NEXT FROM account\_numbers INTO @client\_id, @client\_cpr, @client\_name

END

CLOSE account_numbers
DEALLOCATE account_numbers

Mycroft Holmes

Dammed if I can see any problem with your cursor. Try breaking it down to just print the @client_id for each loop to make sure it is performing correctly. I have an issue with you using the @account_number as a FK between client and account. You should be using the @rec_id as the FK on the client table. Keys should be stupid, you know no intelligence and you are using a formatted string as the key instead of the identity value - bad design!

Never underestimate the power of human stupidity RAH

Jassim Rahma

i did PRINT and it's just printing one record# 25

Corporal Agarn

Try just this

DECLARE account_numbers CURSOR FOR
SELECT clients.client_id, clients.client_cpr, clients.client_name
FROM clients
ORDER BY clients.client_name

OPEN account_numbers

FETCH NEXT FROM account_numbers INTO @client_id, @client_cpr, @client_name

WHILE @@FETCH_STATUS = 0
BEGIN
PRINT @Client_ID

FETCH NEXT FROM account\_numbers INTO @client\_id, @client\_cpr, @client\_name

END

CLOSE account_numbers
DEALLOCATE account_numbers

Also make sure your select statement is returning what you think.

Tim Carmichael

I've had problems with cursors not working correctly, or, telling me the cursor is already open. Can you change from using a cursor to using a temporary table or table variable? Insert the select data with an identity field into the temporary table/table variable and then create a loop to process each record. Hope it helps. Tim

Ben Fair

Could it possibly have anything to do with using GETDATE() to construct 'random' portions of the string? Each pass through the loop will render the same values until the server system clock flips to the next day. It seems to me you will always get the exact same value for each pass of the loop until the next day arrives. I also don't understand your use of the RIGHT() function, perhaps it is some odd way of padding with 0's. In each case it seems you concatenate 000's + some value and then pull the 'some value' you just concatenated from the 000's; why not just use the values themselves and not clutter it up by concatenating the 000's and values together only to turn around and take the values off? If you need to pad try using the REPLICATE() function, as it will be easier to read:

DECLARE @hold varchar(50)
SET @hold = convert(varchar(7), SCOPE_IDENTITY()) -- or @recid
SET @hold = REPLICATE('0', 7 - LEN(@hold)) + @hold -- pad to desired length of 7
-- REPLICATE('0', 0) does not produce an error

Also if you truly need to get a random value try using the NEWID() function which will return a random GUID.

Hold on a second here... Don't you think you might be putting the horse ahead of the cart?