SP won't run as job

[Neil]

I have a stored procedure in SQL 7 which runs fine when called from Query
Analyzer, but fails when it is called as a scheduled job task.

The sp (Customer_UpdateLastEntry_All) gets the last "event" for each
customer from the CustomerEvents table using a cursor (curItems), then loops
through each of those, and, for each one, gets the data from that event
record, using a non-looping cursor (curEvents). Finally, with the data from
curEvents, it calls another sp (Customer_UpdateLastEntry_WithDate), passing
the data to it, which then updates the Customer table based on the input it
receives.

As noted, this sp runs fine when called from QA. However, when run as a job
in SQL Server Agent, I get the message,

"A cursor with the name 'curEvents' does not exist. [SQLSTATE 34000] (Error
16916) A cursor with the name 'curEvents' does not exist. [SQLSTATE 34000]
(Error 16916). The step failed."

The sp and the one it calls are below. Any ideas are appreciated.

Thanks,

Neil

------------------------------------------------------------------------

Alter Procedure Customer_UpdateLastEntry_All

AS
SET NOCOUNT ON

declare @EventID int
declare @CustID int
declare @Date smalldatetime
declare @EntryType varchar(1)
declare @ActivityLogType varchar(50)
declare @Amount float
declare @Notes varchar

declare curItems Cursor Local Forward_Only
For
SELECT Max(ID) AS LastEventID
FROM CustomerEvents
GROUP BY CustID

Open curItems

Fetch Next From curItems
Into @EventID

If @EventID Is Null Goto return_here

While @@Fetch_Status = 0
Begin

declare curEvents Cursor Local Forward_Only
For
SELECT CustID, EntryDate, EntryType, ActivityLogType, Amount, Notes
FROM CustomerEvents
WHERE ID = @EventID

Open curEvents

Fetch Next From curEvents Into @CustID, @Date , @EntryType ,
@ActivityLogType , @Amount , @Notes
If @@Fetch_Status <> 0 Goto on_error

Exec Customer_UpdateLastEntry_WithDate @CustID, @Date , @EntryType ,
@ActivityLogType , @Amount , @Notes

Close curEvents
Deallocate curEvents

Fetch Next From curItems
Into @EventID
End

Close curItems
Deallocate curItems

Close curEvents
Deallocate curEvents

return_here:
Select -1 As ReturnVal
RETURN(0)

on_error:
Select 0 As ReturnVal
RETURN(1)

----------------------------------------------

Alter Procedure Customer_UpdateLastEntry_WithDate

@CustID int,
@Date smalldatetime,
@EntryType varchar(1),
@ActivityLogType varchar(50),
@Amount float,
@Notes text

AS

SET NOCOUNT ON

Update Customer
Set LastEntryType =
Case @EntryType
When 'I' Then 'Invoice'
When 'A' Then @ActivityLogType
When 'W' Then 'Want List'
End,
LastEntryAmt=
Case @EntryType
When 'I' Then @Amount
End,
LastEntryNotes=
Case
When @EntryType= 'A' or @EntryType= 'W' Then @Notes
End,
LastEntryDate=@Date
Where ([INDEX] = @CustID)
IF @@ERROR <> 0 GOTO On_Error

Select -1 As ReturnVal
RETURN(0)

On_Error:
Select 0 As ReturnVal
RETURN(1)

[Stu]

No clue as to why it's failing as a job but not in Query Analyzer;
typically that's a permisions issue, but it doesn't sound like you're
getting a permissions error. I am curious as to why you reverse the
order of your cursor closing, eg:

Close curItems
Deallocate curItems

Close curEvents
Deallocate curEvents

Since you opened curEvents last, and it's nested inside curItems,
wouldn't it make more sense to close it first?

Stu

PS: before anybody else jumps in: "cursors are bad, don't use them,
yada, yada, yada". :)

[Simon Hayes]

I don't know exactly why this creates a problem when scheduled, but you
can get rid of the outer cursor (curItems) by using a derived table,
and then you only have one cursor to manipulate - this may be easier.
Also, @@FETCH_STATUS <> 0 is not necessarily an error, as it usually
just means there are no more rows left in the cursor. So you could try
something like the code below (untested).

Simon

create Procedure dbo.Customer_UpdateLastEntry_All

....
declare curEvents cursor local forward_only
for
select
ce.CustID,
ce.EntryDate,
ce.EntryType,
ce.ActivityLogType,
ce.Amount,
ce.Notes
from dbo.CustomerEvents ce
join
(
SELECT Max(ID) AS 'ID'
FROM dbo.CustomerEvents
GROUP BY CustID
) dt
on ce.ID = dt.ID

Open curEvents

Fetch Next From curEvents Into @CustID, @Date , @EntryType ,
@ActivityLogType , @Amount , @Notes

while @@fetch_status = 0
begin
Exec Customer_UpdateLastEntry_WithDate @CustID, @Date , @EntryType ,
@ActivityLogType , @Amount , @Notes

Fetch Next From curEvents Into @CustID, @Date , @EntryType ,
@ActivityLogType , @Amount , @Notes
end

Close curEvents
Deallocate curEvents

[Erland Sommarskog]

Neil (no****@nospam.net) writes:

I have a stored procedure in SQL 7 which runs fine when called from Query
Analyzer, but fails when it is called as a scheduled job task.

The sp (Customer_UpdateLastEntry_All) gets the last "event" for each
customer from the CustomerEvents table using a cursor (curItems), then
loops through each of those, and, for each one, gets the data from that
event record, using a non-looping cursor (curEvents). Finally, with the
data from curEvents, it calls another sp
(Customer_UpdateLastEntry_WithDate), passing the data to it, which then
updates the Customer table based on the input it receives.

As noted, this sp runs fine when called from QA. However, when run as a
job in SQL Server Agent, I get the message,

I am not surprised, since you close the inner cursor twice. I am more
surprised that you don't get the message when in QA. Anyway, you could
scrap that inner cursor entirely, since you only read once from it
anyway.

But it does not stop there. All that code you posted, can be replace
by this single UPDATE statement:

UPDATE Customer
SET LastEntryType = CASE CE.EntryType
WHEN 'I' Then 'Invoice'
WHEN 'A' Then CE.ActivityLogType
WHEN 'W' Then 'Want List'
END,
LastEntryAmt = CASE CE.EntryType
WHEN 'I' Then CE.Amount
END,
LastEntryNotes = CASE WHEN CE.EntryType IN ('A', 'W')
THEN @Notes
END,
LastEntryDate = CE.Date
FROM Customers C
JOIN CustomerEvents ON C.CustID = CE.CustID
JOIN (SELECT ID = MAX(ID)
FROM CustomerEvents
GROUP BY CustID) AS M ON CE.EventID = M.ID
Not only it's less code. It's going to run faster than the current
solution.
--
Erland Sommarskog, SQL Server MVP, es****@sommarskog.se

Books Online for SQL Server SP3 at
http://www.microsoft.com/sql/techinf...2000/books.asp

[Neil]

"Stu" <st**************@gmail.com> wrote in message
news:11**********************@g43g2000cwa.googlegr oups.com...

No clue as to why it's failing as a job but not in Query Analyzer;
typically that's a permisions issue, but it doesn't sound like you're
getting a permissions error. I am curious as to why you reverse the
order of your cursor closing, eg:

Close curItems
Deallocate curItems

Close curEvents
Deallocate curEvents

Since you opened curEvents last, and it's nested inside curItems,
wouldn't it make more sense to close it first?
Actually, I'm not sure why that Close curEvents is there at the end anyway.
It's actually closed after it's used within the curItems loop. So, by the
time it gets to the end there, it should already be closed and de-allocated.
Maybe that's the problem -- the cursor is already de-allocated, and I'm
telling it to close it.

....

OK, I changed the sp to take out that second Close/Deallocate curEvents at
the end (after the one already in the loop), and -- no more error! So I
guess that was it.

Still doesn't explain why I didn't get an error is QA -- except, perhaps,
that QA handles these things differently. But you'd think that an SP would
be an SP regardless of where it's called from.

Anyway, thanks!

Neil


Stu

PS: before anybody else jumps in: "cursors are bad, don't use them,
yada, yada, yada". :)

[Neil]

"Simon Hayes" <sq*@hayes.ch> wrote in message
news:11*********************@o13g2000cwo.googlegro ups.com...

I don't know exactly why this creates a problem when scheduled,
Yeah, neither do I. See other post in this thread for reason for error;
still doesn't explain why QA didn't generate one, though.
but you
can get rid of the outer cursor (curItems) by using a derived table,
and then you only have one cursor to manipulate - this may be easier.
Yes, that makes more sense.
Also, @@FETCH_STATUS <> 0 is not necessarily an error, as it usually
just means there are no more rows left in the cursor.
In this case, though, it does mean there's an error. curItems is a list of
the most recent item in the CustomerEvents table for each customer.
curEvents is opened for that single item one customer at a time. Thus, if
curEvents doesn't bring back a record, when it was opened with the
criterion:
WHERE ID = @EventID

then there's an error.
So you could try
something like the code below (untested).

Thanks!

Neil

Simon

create Procedure dbo.Customer_UpdateLastEntry_All

...
declare curEvents cursor local forward_only
for
select
ce.CustID,
ce.EntryDate,
ce.EntryType,
ce.ActivityLogType,
ce.Amount,
ce.Notes
from dbo.CustomerEvents ce
join
(
SELECT Max(ID) AS 'ID'
FROM dbo.CustomerEvents
GROUP BY CustID
) dt
on ce.ID = dt.ID

Open curEvents

Fetch Next From curEvents Into @CustID, @Date , @EntryType ,
@ActivityLogType , @Amount , @Notes

while @@fetch_status = 0
begin
Exec Customer_UpdateLastEntry_WithDate @CustID, @Date , @EntryType ,
@ActivityLogType , @Amount , @Notes

Fetch Next From curEvents Into @CustID, @Date , @EntryType ,
@ActivityLogType , @Amount , @Notes
end

Close curEvents
Deallocate curEvents

[Neil]

"Erland Sommarskog" <es****@sommarskog.se> wrote in message
news:Xn**********************@127.0.0.1...

Neil (no****@nospam.net) writes:

I have a stored procedure in SQL 7 which runs fine when called from Query
Analyzer, but fails when it is called as a scheduled job task.

The sp (Customer_UpdateLastEntry_All) gets the last "event" for each
customer from the CustomerEvents table using a cursor (curItems), then
loops through each of those, and, for each one, gets the data from that
event record, using a non-looping cursor (curEvents). Finally, with the
data from curEvents, it calls another sp
(Customer_UpdateLastEntry_WithDate), passing the data to it, which then
updates the Customer table based on the input it receives.

As noted, this sp runs fine when called from QA. However, when run as a
job in SQL Server Agent, I get the message,
I am not surprised, since you close the inner cursor twice.

Yes, you're right. Per other post in this thread, that was the reason for
the error.
I am more
surprised that you don't get the message when in QA.
Yes, me too.
Anyway, you could
scrap that inner cursor entirely, since you only read once from it
anyway.
Meaning make the SQL for getting the Max ID a derived table and loop through
a single cursor, or did you mean something else?

But it does not stop there. All that code you posted, can be replace
by this single UPDATE statement:

UPDATE Customer
SET LastEntryType = CASE CE.EntryType
WHEN 'I' Then 'Invoice'
WHEN 'A' Then CE.ActivityLogType
WHEN 'W' Then 'Want List'
END,
LastEntryAmt = CASE CE.EntryType
WHEN 'I' Then CE.Amount
END,
LastEntryNotes = CASE WHEN CE.EntryType IN ('A', 'W')
THEN @Notes
END,
LastEntryDate = CE.Date
FROM Customers C
JOIN CustomerEvents ON C.CustID = CE.CustID
JOIN (SELECT ID = MAX(ID)
FROM CustomerEvents
GROUP BY CustID) AS M ON CE.EventID = M.ID
Not only it's less code. It's going to run faster than the current
solution.
Yes, that would be more efficient. Only thing is, I don't like having code
like this in two places. I have it in the SP that is called so that, as a
customer event occurs, the customer's information is updated in real time by
calling the SP for that single customer. Then I have the SP that calls this
SP for all customers, on a nightly basis, in case some glitch happened on
the client side and the SP wasn't called.

Thus, the code needs to remain in the SP for single customers, but then be
called for all customers.

I suppose I could make the SP dual-purpose by passing a parameter with a
customer ID; or, if Null, to modify the SQL to run for all customers. I
suppose that would work, and would have the code in one place as I like, and
be more efficient. That might be the way to go.

Thanks,

Neil


--
Erland Sommarskog, SQL Server MVP, es****@sommarskog.se

Books Online for SQL Server SP3 at
http://www.microsoft.com/sql/techinf...2000/books.asp

[Erland Sommarskog]

Neil (no****@nospam.net) writes:

Yes, that would be more efficient. Only thing is, I don't like having
code like this in two places. I have it in the SP that is called so
that, as a customer event occurs, the customer's information is updated
in real time by calling the SP for that single customer. Then I have the
SP that calls this SP for all customers, on a nightly basis, in case
some glitch happened on the client side and the SP wasn't called.
Well, if the number of customers to update is small, it might be OK. If you
have a million, the job could be running all night. Instead of in a minute.
I suppose I could make the SP dual-purpose by passing a parameter with a
customer ID; or, if Null, to modify the SQL to run for all customers. I
suppose that would work, and would have the code in one place as I like,
and be more efficient. That might be the way to go.

But don't say:

WHERE custid = @custid OR @custid IS NULL

because then the single updates cannot use the index.

You can use a temp table or a table variable, but it is still the case that
the nightly update and the single-update will need different query plans.
From the point of view of performance, the best may be to have an "IF
@custid IS NULL", and then have two UPDATE statements in the same procedure,
so at least that they are close to each other.

When it comes to code reuse, the rules change when you are in a database,
at least when the language is T-SQL. (Oracle's PL/SQL is reportedly more
modern, but how it works in practice, I don't know.) So one has to raise
the bar on when to avoid duplicating logic a bit.
--
Erland Sommarskog, SQL Server MVP, es****@sommarskog.se

Books Online for SQL Server SP3 at
http://www.microsoft.com/sql/techinf...2000/books.asp

[Neil]

> Well, if the number of customers to update is small, it might be OK. If

you
have a million, the job could be running all night. Instead of in a
minute.
There are about 20,000 customers, but it doesn't seem to take too long to
run.

I suppose I could make the SP dual-purpose by passing a parameter with a
customer ID; or, if Null, to modify the SQL to run for all customers. I
suppose that would work, and would have the code in one place as I like,
and be more efficient. That might be the way to go.

But don't say:

WHERE custid = @custid OR @custid IS NULL

because then the single updates cannot use the index.

I believe in the past I did something where I passed either a customer ID or
"%" and then used 'Where CustID Like @Param', running it for just the
customer ID or for all customers.

Neil

[Erland Sommarskog]

Neil (no****@nospam.net) writes:

I believe in the past I did something where I passed either a customer
ID or "%" and then used 'Where CustID Like @Param', running it for just
the customer ID or for all customers.

In that case it works well if you add WITH RECOMPILE to the procedure
definition. You will have to take the cost of recompilation each time,
though. Which for a short procedure can be negliglible.
--
Erland Sommarskog, SQL Server MVP, es****@sommarskog.se

Books Online for SQL Server SP3 at
http://www.microsoft.com/sql/techinf...2000/books.asp