SQL for Smarties | SQL Programming Style | Trees and Hierarchies in SQL | SQL Puzzles and Answers | Data and Databases

Monday, June 19, 2006

Cursor not completing when stored procedure runs within it

SQL Apprentice Question
I am having an interesting problem I haven't seen.

First, here's the code that sets up the cursor, with a select statement
where the exec should be, and the results:

DECLARE @order_id int,
@row_id int,
@qty_rtn int,
@invoice_id int,
@date_shipped datetime

select r.order_id_display, r.row_id -1, r.quantity, s.line_id,
getdate() from batch..temp_response r, shipment s,
receipt_item i
where isnull(r.status, 0) >= 0 and new_status in ('R', 'U')
and i.i_order_id_display = r.order_id_display
and i.order_id = s.order_id
and i.row_id = r.row_id - 1
and i.upc=r.upc and amount = 1
and i.order_id in ('0FD94RQXB4JL9J8V4R3G5B8CC5') --for
testing purposes I selected one order only

OPEN order_return
FETCH NEXT FROM order_return INTO @order_id, @row_id, @qty_rtn,
@invoice_id, @date_shipped
select 'exec process_line_item_shipping', @order_id, @row_id, 0,
@qty_rtn, @date_shipped, @invoice_id
-- exec process_line_item_shipping @order_id, @row_id, 0, @qty_rtn,
@date_shipped, @invoice_id

FETCH NEXT FROM order_return INTO @order_id, @row_id, @qty_rtn,
@invoice_id, @date_shipped

CLOSE order_return
DEALLOCATE order_return

This returns

exec process_line_item_shipping 491232 0 0 1
2006-06-16 12:46:19.330 534386
exec process_line_item_shipping 491232 1 0 1
2006-06-16 12:46:19.330 534386

Which is exactly what I'd expect.

HOWEVER.... when I remove the comment tag off the actual SP exec
command, then I ONLY get
exec process_line_item_shipping 491232 0 0 1
2006-06-16 12:46:19.330 534386

and only the first exec statement runs.

I've done a select @@fetch_status before and after the exec statement,
and it's 0 each time.

The stored procedure run has no cursors within it, just several
calculations, inserts and update statements.

Celko Answers
Please post DDL, so that people do not have to guess what the keys,
constraints, Declarative Referential Integrity, data types, etc. in
your schema are. Sample data is also a good idea, along with clear
specifications. It is very hard to debug code when you do not let us
see it.

What you did post was awful. You are using SQL cursors, which are the
worst way to use SQL -- orders of magnitude poorer performance, lack of
portability, etc. Read some of the postings here and *any* other SQL
Newsgroup. My rule of thumb is that you should not write more than
five of them in 25 years in IT.

Looking at what you did post, it looks like you missed most of the
basic ideas of RDBMS and building a procedural routine that mimics a
file system. .

1) Why would anyone put the display order into a table? All display
work is done in the front end and not the database.

2) Ignoring design flaw #1, why did you use two different names for the
same data element (I.i_order_id_display = R.order_id_display)? Surely
nobody would put the data type or table on a data element.

3) What is a row_id? If it refers to the physical rows in a table,
then it is wrong. If it refers to the position on the input screen or
original paper form, then it is wrong. You woudl be mimicing a paper
form instead of building a relational model.

4) You use vague data element names Amount of what? It does not seem
to be money. Quantity of what? Ordered or returned or on-hand, or what?
That is like an adjective without a noun.

5) Why don't you follow ISO-11179 naming rules or at least be
consistent? Look at @date_shipped is "" while @invoice_id
is "" instead.

6) When I see procedure named "Process_Line_Item_Shipping' I worry
that you are going thru each item in an order, one at a time. SQL is a
set-oriented language and you should be working with a sub-set of
items. No loops. No Cursors.

My guess, based on no DDL, is that you need a table for the Orders, for
the Order Details, Shipments and working table of returns. The
returns will be used to update the Order Details with return
quantities and shipping info (perhaps the Orders will need changes).

I have done this in one UPDATE statement for some fairly simple
business rules. The trick was a detail table keyed on (order_nbr, sku,
ship_status, ship_date). Reports are done off of VIEWs (what
percentage of Lawn Gnomes are returned? in how many days? ) and you
needed to watch constraints (you cannot return more than you bought).

No comments: