SQL Screw-ups: Stored Procedure Misuse

This is a continuation of the SQL Screw-ups series that stemmed from my Nashville .NET User Group talk on 03/14/2019. Slides and setup details are on the first post in the series.

Imagine you’re starting a brand new project. You can make design decisions to mitigate all the issues you encountered in the legacy app you’ve been stuck working on. What a great opportunity! While the new project must integrate with the legacy app, none of the code has to match the legacy style.

One of the first decisions you make is to exclusively use stored procedures for all data interaction in the app. This is a reasonable choice because stored procedures protect your code and they are faster than other methods of executing queries because they are compiled on the server. The internets said to use stored procedures. Your friends ridicule you because your previous project didn’t use stored procedures. You won’t make that mistake again.

You get to work on your new app. You create a couple of tables for setup information and three stored procedures per table: create, read, and update. You don’t write stored procedures to delete records because you need to maintain referential integrity even if records aren’t valid to use in the future. You go ahead and flesh out your data transfer objects, repositories, models, view models, and views. Things are really coming together!

Here is an example of a query you wrote to read all online orders in a date range for a customer.

SELECT *
FROM Sales.SalesOrderHeader
WHERE OrderDate >= '2013-07-01'
  AND OrderDate <  '2014-07-01'
  AND OnlineOrderFlag = 1
  AND CustomerID = 11091
ORDER BY SalesOrderID;

You create a stored procedure from it:

CREATE PROCEDURE Sales.GetOrdersForCustomerInDateRange 
    @OrderDateRangeBegin datetime2(7) = NULL
   ,@OrderDateRangeEnd   datetime2(7) = NULL
   ,@OnlineOrderFlag     bit          = 0
   ,@CustomerID          int          = NULL
AS
BEGIN
    SELECT *
    FROM Sales.SalesOrderHeader
    WHERE OrderDate >= @OrderDateRangeBegin
      AND OrderDate <  @OrderDateRangeEnd
      AND OnlineOrderFlag = @OnlineOrderFlag
      AND CustomerID = @CustomerID
    ORDER BY SalesOrderID;
END

You execute the stored procedure, it works well for your prototype, and you’re very pleased!

EXEC Sales.GetOrdersForCustomerInDateRange
    @OrderDateRangeBegin = '2013-07-01'
   ,@OrderDateRangeEnd   = '2014-07-01'
   ,@OnlineOrderFlag     = 1
   ,@CustomerID          = 11091

See the github repository at the end of this post for the sample application code that executes the SQL in this post..

The project sponsor looks at your prototype and wants to filter on order dates, but not for a specific customer. Your stored procedure returns zero records when you send null to the customer ID because there are no orders with a null customer.

EXEC Sales.GetOrdersForCustomerInDateRange
    @OrderDateRangeBegin = '2013-07-01'
   ,@OrderDateRangeEnd   = '2014-07-01'
   ,@OnlineOrderFlag     = 1
   ,@CustomerID          = NULL

At this point, you have a couple of options. One option is to write a separate stored procedure with no customer parameter and decide which stored procedure to call from your application code based on the filters you have. Sounds tedious. Another option is to account for a null parameter in the query so null parameters are ignored. You choose this option in an effort to keep your application code a little cleaner.

Here is your modified stored procedure. It will work if any parameters are null. This offers the most flexibility for your stored procedure, and you are pleased.

CREATE PROCEDURE Sales.GetOrders
    @OrderDateRangeBegin datetime2(7) = NULL
   ,@OrderDateRangeEnd   datetime2(7) = NULL
   ,@OnlineOrderFlag     bit          = NULL
   ,@CustomerID          int          = NULL
AS
BEGIN
    SELECT *
    FROM Sales.SalesOrderHeader
    WHERE OrderDate >= ISNULL(@OrderDateRangeBegin, '0001-01-01')
      AND OrderDate <  ISNULL(@OrderDateRangeEnd  , '9999-12-31')
      AND OnlineOrderFlag = ISNULL(@OnlineOrderFlag, OnlineOrderFlag)
      AND CustomerID = ISNULL(@CustomerID, CustomerID)
    ORDER BY SalesOrderID;
END

So that’s it! You’ve successfully accomplished your goal of using stored procedures in your new app! Congratulations!

But wait, there’s more …

As you flesh out your prototype more, you start having to integrate with the legacy app. Remember that part from the original requirements? Does anyone anticipate this going downhill?

In the legacy app to which you contribute exists very cleverly designed and convenient filter controls for the UI that dynamically build a SQL predicate. The predicate then gets tacked onto the end of a query for filtering. The handy part of these filter controls is that you can use them anywhere, and they work with all the things, provided you’ve set your core query up to work with them. Fine, you’ll encounter runtime exceptions here and there when a developer doesn’t fully test all permutations of a filter, and that one table that gets filtered isn’t joined … but you decided long ago that the convenience was well worth the risk, and you get to reuse the filtering code in a LOT of places.

Now, though, you must support the output from those filter controls in your new app. Cool, you think, we’ll just tack the predicate on the end of our SQL like we did in the legacy app. Wrongo! How you gonna tack SQL onto the end of a stored procedure that lives on the server? So you Google it and find a way. Here’s a bit of truth. If it’s a bad idea, Google will tell you how to implement it. Now this isn’t inherently a bad idea, but you decide to implement it in an incremental way. That is, you convert your existing stored procedure into a version of the stored procedure that does exactly the same thing your previous version did, except it now allows you to tack on a predicate.

The approach you take is to build a query as a string in your stored procedure, then execute that string. The concept is very simple, but notice that we have to escape any single quotes in our query.

DECLARE @query nvarchar(max) = N'SELECT ''World!'' as Hello'
exec(@query)

Here is your converted stored procedure. Note that it still supports filtering on any combination of columns for which you offer parameters – you didn’t lose any functionality. But now, it also supports predicates built by other areas of the system that apply to your query.

CREATE PROCEDURE Sales.GetOrdersDynamic
    @OrderDateRangeBegin datetime2(7)  = NULL
   ,@OrderDateRangeEnd   datetime2(7)  = NULL
   ,@OnlineOrderFlag     bit           = NULL
   ,@CustomerID          int           = NULL
   ,@WildWestSql         nvarchar(max) = NULL
AS
BEGIN
  DECLARE @query nvarchar(max) = N'
    SELECT *
    FROM Sales.SalesOrderHeader
    WHERE OrderDate >= ''' + CONVERT(nvarchar, ISNULL(@OrderDateRangeBegin, N'1973-01-01'), 120) + N'''
      AND OrderDate <  ''' + CONVERT(nvarchar, ISNULL(@OrderDateRangeEnd  , N'9999-12-31'), 120) + N'''
      AND OnlineOrderFlag = ISNULL(' + CASE WHEN @OnlineOrderFlag IS NULL THEN N'NULL' WHEN @OnlineOrderFlag = 0 THEN N'0' ELSE N'1' END + N', OnlineOrderFlag)
      AND CustomerID = ISNULL(' + CASE WHEN @CustomerID IS NULL THEN N'NULL' ELSE CONVERT(nvarchar, @CustomerID) END + N', CustomerID)';

  IF @WildWestSql IS NOT NULL
    SET @query = @query + N' AND ' + @WildWestSql + N' ';

  SET @query = @query + N'ORDER BY SalesOrderID;'
  exec(@query);
END

You can test your stored procedure in SSMS:

EXEC Sales.GetOrdersDynamic DEFAULT, DEFAULT, 1, DEFAULT, 'Sales.SalesOrderHeader.CustomerID IN (SELECT Sales.Customer.CustomerID FROM Sales.Customer WHERE Sales.Customer.TerritoryID = 10)'

You then notice some performance issues during user acceptance testing. After reaching out to some people in person at your local user group about your problem, you get ample and concise feedback about what is likely happening. A side-effect is that you get to know some people in your local development community, realize they know what they’re talking about, and they are really nice people. From your professional networking exercise, you learn that SQL Server is coming up with an execution plan for the stored procedure on first run and using that execution plan for all subsequent runs regardless of the parameter values. This results in multiple full table scans every time the stored procedure executes. What the heck?! Stored procedures are supposed to be faster, but they’re slowing your system to a crawl. You do more research on this problem, and you decide to ask the community for help with a solution. [In my case, I’ll ask a SQL expert from my local user group to write a guest blog post that explains this problem and some solutions.]

One solution is to dynamically build the where clause in the stored procedure so you’re only executing predicates that are needed.

CREATE PROCEDURE Sales.GetOrdersDynamicPlus
    @OrderDateRangeBegin datetime2(7)  = NULL
   ,@OrderDateRangeEnd   datetime2(7)  = NULL
   ,@OnlineOrderFlag     bit           = NULL
   ,@CustomerID          int           = NULL
   ,@WildWestSql         nvarchar(max) = NULL
AS
BEGIN
  DECLARE @query nvarchar(max) = N'
    SELECT *
    FROM Sales.SalesOrderHeader
    WHERE 1=1'
  IF @OrderDateRangeBegin IS NOT NULL
    @query = @query + N'
      AND OrderDate >= ''' + CONVERT(nvarchar, ISNULL(@OrderDateRangeBegin, N'1973-01-01'), 120) + N''''

  IF @OrderDateRangeEnd IS NOT NULL
    @query = @query + N'
      AND OrderDate <  ''' + CONVERT(nvarchar, ISNULL(@OrderDateRangeEnd  , N'9999-12-31'), 120) + N''''

  IF @OnlineOrderFlag IS NOT NULL
    @query = @query + N'
      AND OnlineOrderFlag = ISNULL(' + CASE WHEN @OnlineOrderFlag IS NULL THEN N'NULL' WHEN @OnlineOrderFlag = 0 THEN N'0' ELSE N'1' END + N', OnlineOrderFlag)'

  IF @CustomerID IS NOT NULL
    @query = @query + N'
      AND CustomerID = ISNULL(' + CASE WHEN @CustomerID IS NULL THEN N'NULL' ELSE CONVERT(nvarchar, @CustomerID) END + N', CustomerID)';

  IF @WildWestSql IS NOT NULL
    @query = @query + N' AND ' + @WildWestSql + N' ';

  @query = @query + N'ORDER BY SalesOrderID;'
  exec(@query);
END

SQL then sees your query as what it really is, rather than the same query running differently with the same execution plan.

EXEC Sales.GetOrdersDynamicPlus DEFAULT, DEFAULT, 1, DEFAULT, 'Sales.SalesOrderHeader.CustomerID IN (SELECT Sales.Customer.CustomerID FROM Sales.Customer WHERE Sales.Customer.TerritoryID = 10)'

At the end of the day, you decide that stored procedures really just don’t mesh well with the way your team and department work with this particular app. Because the entire team is accustomed to seeing the SQL alongside the application code, you decide to migrate back to building dynamic SQL in your application code so everything is discoverable in one place for your team. There are many arguments for using stored procedures, but perhaps the best argument against using stored procedures is if it creates a divide or barrier for your team to effeciently and effectively develop and maintain the product. Using stored procedures for a portion of our product did not add value in our case, it only added cost in the forms of both time spent and frustration among team members.

Check out the sample application code that goes along with this post in my github repository.
https://github.com/evansmithdev/BlogPosts/tree/master/SQL-Screwups-2019-03-14

1 thought on “SQL Screw-ups: Stored Procedure Misuse

  1. Pingback: SQL Screw-ups: Scalar-Valued Functions – Evan Smith

Comments are closed.