SQL cursors and dynamic SQL

Question

The idea is to have a heading for each date returned by the query. This could be any number of dates. I dynamically altered the #table which is ugly but there is no other way.

Ultimately I would like to reduce the amount of cursors but I'm not sure if it is possible. This is using the Northwind database on SQL Server 2000 (I know, its lifecycle has ended).

  USE Northwind

  DECLARE @startdate DATETIME, @enddate DATETIME, @sql VARCHAR(1000)
  SELECT @startdate = '1998-01-01', @enddate = '1998-06-01'


  DROP TABLE #OUTPUT
  DROP TABLE #TEMP

  CREATE TABLE #OUTPUT 
  (
    product VARCHAR(50)
  )


  SELECT o.orderdate, p.ProductName
  INTO #TEMP
  FROM [Northwind].[dbo].[Orders] o
  join [order details] od ON od.orderid = o.orderid
  join Products p on p.ProductID = od.ProductID
  WHERE o.OrderDate >= @startdate AND o.OrderDate < @enddate

  --Cursor storage variables
  DECLARE @prod VARCHAR(30), @orderdate VARCHAR(20), @cnt AS INT
  --

  --Add date column headings
  DECLARE CUR1 CURSOR LOCAL FOR
    SELECT DISTINCT(CONVERT(VARCHAR, orderdate, 101)) FROM #TEMP

  OPEN CUR1     
  FETCH NEXT FROM CUR1 INTO @orderdate 

  WHILE @@FETCH_STATUS = 0 
  BEGIN
    EXEC('ALTER TABLE #OUTPUT ADD [' + @orderdate + '] VARCHAR(20) DEFAULT 0')  
    FETCH NEXT FROM CUR1 INTO @orderdate
  END
  --

  --Add left column containing list of products
  INSERT INTO #OUTPUT (product)
  SELECT DISTINCT(productname) FROM #TEMP 
  --

  --Update all rows setting the approriate date column to the count
  DECLARE CUR2 CURSOR LOCAL FOR
    SELECT CONVERT(VARCHAR, orderdate, 101), productname, COUNT(*) as cnt 
    FROM #TEMP
    GROUP BY CONVERT(VARCHAR, orderdate, 101), productname

  OPEN CUR2

  FETCH NEXT FROM CUR2 INTO @orderdate, @prod, @cnt

  WHILE @@FETCH_STATUS = 0
  BEGIN
    SELECT @sql = 'UPDATE #OUTPUT set [' + @orderDate + '] =  [' + @orderdate + '] + ' + CAST(@cnt AS VARCHAR) + '  WHERE LTRIM(RTRIM(product)) = ''' + LTRIM(RTRIM(REPLACE(@prod, '''', ''))) + ''' '  
    EXEC(@sql)
    PRINT @sql
    FETCH NEXT FROM CUR2 INTO @orderdate, @prod, @cnt   
  END

  SELECT * FROM #OUTPUT

The result should look like this:

product           01/01/1998    01/02/1998  
Alice Mutton           0            2
Aniseed Syrup          3            1
Boston Crab Meat       1            2

Answer 1

You stated you are stuck with SQL Server 2000, which sucks; but with that in mind, here are my thoughts. Bear in mind, I'm not a SQL Server specialist, and other answers may provide a more appropriate solution.

Good things

Your SQL syntax & indentation are great. There are a few keywords that are in different case, but overall I think you are good. Your logic is easy to follow. There are a lot of missing ; statement terminators but SQL Server is very permissive. I recommend you always use terminators. Also, your single letter aliases could be improved. I will include that in my code below the next section.

But now, I must address your...

Approach

What you are essentially doing is a pivot. SQL Server 2000 is an outdated product, and there are much more appropriate solutions available with newer versions, which require neither a CURSOR nor dynamic SQL. That said, I strongly question the usefulness of the result data set.

If you're querying only a few days, sure; but, imagine a query like that over a year or longer. You would be stuck with 365+ columns of data to look through rows for each product. It would be much more useful if the data was not pivoted, i.e., have a column for products and a column for dates. It is much easier to graph using Excel and such. And pivoting the data is a trivial task in Excel.

SQL really sucks at looping, and a cursor does exactly that which sucks. My opinion is that you would be better off just getting the data and not locking down your database server while this cursor is working to rearrange how the data is displayed to pivot it. Here is an example query using Northwind:

  USE Northwind;

  DECLARE @startdate DATETIME;
  DECLARE @enddate DATETIME;
  SET @startdate = '1998-01-01';
  SET @enddate = '1998-06-01';

  SELECT 
      ord.orderdate, 
      prod.ProductName,
      COUNT(ordDetail.Quantity) AS [Qty Sold]
  FROM [Northwind].[dbo].[Orders] AS ord
    JOIN [order details] AS ordDetail 
       ON ordDetail.orderid = ord.orderid
    JOIN Products AS prod 
       ON prod.ProductID = ordDetail.ProductID
  WHERE ord.OrderDate >= @startdate 
    AND ord.OrderDate < @enddate
  GROUP BY [Qty Sold],
      ord.orderdate,
      prod.ProductName;

And just let the user pivot and manipulate it as they see fit. The purpose of a database system is to manage data, not manage how data is presented.

Answer 2

About dynamic columns

Making a query return dynamic columns is a bad idea. This kind of operation requires looping, which is not a design goal of databases. There are situations when looping and cursors lead to massive speed benefits, and then acceptable, but this is not such case.

It's an interesting (but dirty) hack that you could make this work, but a misuse of databases. It would be better to return the data with fixed columns (product, date, count) and let client applications present that as they like (as @Phrancis suggested it too).

Optimize and simplify

It will be more efficient to add the count in the #TEMP table from the start. And the rest of the code will be simpler if order date is also converted to the right format at this point:

  SELECT CONVERT(VARCHAR, o.OrderDate, 101) AS OrderDate, p.ProductName, COUNT(*) AS Cnt
  INTO #TEMP
  FROM [Northwind].[dbo].[Orders] o
  JOIN [order details] od ON od.orderid = o.orderid
  JOIN Products p ON p.ProductID = od.ProductID
  WHERE o.OrderDate >= @startdate AND o.OrderDate < @enddate
  GROUP BY o.OrderDate, p.ProductName

This will be more optimal, because the #TEMP table doesn't have to contain all the records, only the aggregates with count.

This will also simplify this cursor declaration:

  DECLARE CUR1 CURSOR LOCAL FOR
    SELECT DISTINCT(CONVERT(VARCHAR, orderdate, 101)) FROM #TEMP

To this:

  DECLARE CUR1 CURSOR LOCAL FOR
    SELECT OrderDate FROM #TEMP

Likewise, the other cursor becomes much simpler too:

  DECLARE CUR2 CURSOR LOCAL FOR
    SELECT OrderDate, ProductName, Cnt 
    FROM #TEMP

Coding style

Even when names are case insensitive, it makes the code more readable to use the same writing style everywhere. So instead of sometimes using OrderDate and sometimes using orderdate, choose one style and stick to it.

It's also customary to write all SQL keywords in uppercase. You did that for most, but not all, for example JOIN and ON.