Philosophy on stored procedures that perform updates: is a 'granular' update approach sufficient?

Question

I am embarking on on a large project and have an inquiry about stored procedures in general.

Let's say I have a User table.

UserKey IDENTITY NOT NULL PRIMARY KEY
FirstName nvarchar(50)NOT NULL
LastName nvarchar(50) NOT NULL
Email nvarchar(50) NOT NULL
Birthdate date NULL
EffectiveFrom datetimeoffset NOT NULL
EffectiveThru datetimeoffset NULL

In reality, there are some more fields here, but for the sake of brevity I have skimmed it down to the basics.

Now, I am faced with creating stored procedures to allow basic CRUD functionality for this table.

The Insert sproc is simple enough.

usp_INSERT_User:

CREATE PROCEDURE [dbo].[usp_INSERT_User]
(
    --@UserKey BIGINT,
    @FirstName NVARCHAR(50),
    @LastName NVARCHAR(50),
    @Email NVARCHAR(50),
    @BirthDate DATE = NULL,
    @EffectiveFromDtTm DATETIMEOFFSET(7),
    @EffectiveThruDtTm DATETIMEOFFSET(7) = NULL,
)

My main inquiry is how I should write an update sproc.

Since there can be pretty granular updates, should I create an update stored procedure for each of those granular updates?

For example, let's say I only want to update the user's email after processing a verified email update for the user, would I write a sproc with one parameter?

usp_UPDATE_UserEmail @NewEmail

If the user updated their profile, I could write a sproc like:

usp_UPDATE_User @UserKey, @FirstName, @LastName, @Email, @Birthdate

So, to cover my most common update operations, I might end up with a few sprocs to cover use cases of updates to the User table. Obviously, the larger the table and/or the higher number of business requirements would have a direct correlation to the number of update sprocs written for a particular table. This gets me thinking about maintainability of a ton of sprocs.

Yesterday, I went down the rabbit hole of

"oh, why don't I just write ONE end-all-be-all update sproc that can take whatever parameter I want to update via an XML param"

Luckily, without using dynamic sql, I was able to create that monstrosity. And it works.

I then decided to test speeds between my fancy xml sproc and just a regular update sproc (with multiple params) with very similar updates.

So, I created a mock user table with 12 million records and tested both approaches. Unsurprisingly, the XML approach on average, took 7x longer than the vanilla update sproc.

There is also additional overhead on the other side of the wire serializing parameters into XML, which could be of concern with a large update batch.

Now, I say all of that to wrap up and ask which type of approach in your experience is most often (if at all) seen? Is is most common practice to have granular sprocs like usp_UPDATE_Field1 / usp_UPDATE_Field2 / usp_UPDATE_Field3&4 OR usp_UPDATE_whateveryouwant

I like the simplicity of having a golden sproc that can take however many number of parameters you want to pass it and update them accordingly, but there are also performance drawbacks.

To give more context: running SQL Server 2019. I am also pretty new at this stuff.

Thanks for any help!

Answer 1

Stored procs are the programming API of the database, the methods you invoke to mutate the system state. As such, I prefer to have SPs names reflect the business outcome desired rather than the implementation - change_of_address rather than update_customer. Each SP has the parameters to perform that business change and no others. Each does one thing and does it well.

If the business supports two processes that both update a common column then so be it. That column is an input to each of those SPs. Say there's a web based "change profile" and a mobile-only "register new phone". They both write to customer.phone_number. I would have separate SPs for each business purpose.

There is no measurable overhead to having many small, tightly focused stored procedures. Changes to data types are automatic on the next recompile. Changes to column names are simple search-and-replace. SSMS's built-in dependency analysis can help, too. If you use local variables that mirror columns use the column's name e.g. customer_name is read into @old_customer_name.

Answer 2

Personally I don't understand why people write stored procedures that only perform CRUD. For me SQL is the interface to the database. Just write an update statement with placeholders for the column values and send that to the database -- along with the bound variables.

Someone will argue about SQL injection, but a) you're just moving the point where that can happen (the stored procedure could still be written badly), and b) you still have to guard for other forms of injection (eg a malicious user entering HTML and JavaScript in a text field).

Answer 3

I would definitely avoid the complication of packing and unpacking XML, effectively turning a single input parameter into a DSL. It is something extra that can have bugs at either end (and a bug is in the calling code, you may end up having to prove it isn't in your DB-side code before that is accepted!).

Would extra parameters be acceptable for columns that can be set to NULL? If so then you could do something like:

CREATE OR ALTER PROCEDURE update_some_table (
      @recordId BIGINT
    , @col1 NVARCHAR(50) = NULL
    , @col2 NVARCHAR(50) = NULL
    , ...
    , @col1NULL BIT = 0
    , @col2NULL BIT = 0
    , ...
) AS
BEGIN
    UPDATE some_table
    SET    col1 = CASE 
                  WHEN @col1 IS NOT NULL THEN @col1 
                  WHEN @col1NULL = 1     THEN NULL
                  ELSE col1 -- no new value and no instruction to NULL, so keep present value
         , col1 = CASE 
                  WHEN @col2 IS NOT NULL THEN @col2
                  WHEN @col2NULL = 1     THEN NULL
                  ELSE col2 -- no new value and no instruction to NULL, so keep present value
         , ...
    WHERE  id = @recordId
END

Then you can call this with a mix of patterns:

EXEC update_some_table @recordId = 1, @col1 = 'new value'
EXEC update_some_table @recordId = 1, @col1 = 'new value', @col2 = 'another new value'
EXEC update_some_table @recordId = 1, @col2 = 'another new value'
EXEC update_some_table @recordId = 1, @col1NULL = 1, @col2 = 'another new value'
EXEC update_some_table @recordId = 1, @col2NULL = 1

In any case there is only one search for the row to update rather than potentially having one update statement per property (which might be part of why your solution is slower, along with the XML unpacking jigger-pokery).

I think (I've not checked) that this will result in the columns always looking to be updated if you inspect the relevant bits of COLUMNS_UPDATED inside a trigger that fires because of this procedure, which may be a consideration if you are using trigger based auditing.

Of course if all you are doing is forming an UPDATE statement from the provided parameters, you just let the application issue its own UPDATE statement (using constraints, triggers, and perhaps temporal tables, for any validation and auditing requirements)?

Answer 4

The "granular updates" question is orthogonal to the question of whether to put it in a stored procedure, as you can break it down both in client code or in a stored procedure.

I must say, I only ever see a point to breaking down update queries like this when dealing with very wide and/or heavily indexed tables, or where triggers are involved. The point being that a column in the UPDATE statement is considered an update even if no change happened.

In such a situation, I would actually tend towards dynamic SQL. Build up the query in your favourite client-side language, using proper parameterization, and execute. It is pretty trivial, for example, in C#.

// class User {

public async Task Update(string connectionString)
{
    SqlParameters[] params = GetColsToUpdate();
    var query = $@"
UPDATE User
SET
{string.Join("/r/n", params.Select(p => $"{p.ParameterName} = @{p.ParameterName},"))}
WHERE UserKey = @UserKey;
";
    using(var conn = new SqlConnection(connectionString))
    using(var comm = new SqlCommand(conn, query))
    {
        comm.Parameters.AddRange(params);
        comm.Parameters.Add("@UserKey", SqlDbType.BigInt).Value = UserKey;
        await conn.OpenAsync();
        await comm.ExecuteNonQueryAsync();
    }
}

It does have the side-effect of recompilation and adding more queries to the plan cache, but that is actually the point: we may want different plans for different updated columns. What we don't want, and won't get, is a different plan for every actual value being passed in.

If you want to restrict yourself to a stored procedure, you could do perhaps something like this:

CREATE PROCEDURE [dbo].[User_UPDATE]
(
    @ColumnsToUpdate OneColumnSysnameTableType,

    @UserKey BIGINT,
    @FirstName NVARCHAR(50) = NULL,
    @LastName NVARCHAR(50) = NULL,
    @Email NVARCHAR(50) = NULL,
    @BirthDate DATE = NULL
)
AS

DECLARE @sql nvarchar(max) = N'

UPDATE dbo.User
SET
' +
(
SELECT STRING_AGG(CAST(v.name + N' = ' + v.var AS nvarchar(max)), N'
    ')
FROM (VALUES
  ('UserKey',  '@UserKey'),
  ('FirstName','@FirstName'),
  ('LastName', '@LastName'),
  ('Email',    '@Email'),
  ('BirthDate','@BirthDate')
) v(name, var)
JOIN @ColumnsToUpdate c ON c.val = v.name
);

EXEC sp_executesql @sql,
  N'@UserKey BIGINT,
    @FirstName NVARCHAR(50),
    @LastName NVARCHAR(50),
    @Email NVARCHAR(50),
    @BirthDate DATE
',
    @UserKey = @UserKey,
    @FirstName = @FirstName ,
    @LastName = @LastName,
    @Email = @Email,
    @BirthDate = @BirthDate;


GO

As you can see, it is a lot of boiler-plate, and probably should itself be generated via a script.