9

I have a production SQL-Server DB (reporting) that has many Stored Procedures. The SPs are publicly exposed to the external world in different ways
- some users have access directly to the SP,
- some are exposed via a WebService
- while others are encapsulated as interfaces thru a DCOM layer.

The user base is large and we do not know exactly which user-set uses which method of accessing the DB.
We get frequent (about 1 every other month) requests from user-sets for modifying an existing SP by adding one column to the output or a group of columns to the existing output, all else remaining same.
We initially started doing this by modifying the existing SP and adding the newly requested columns to the end of the output. But this broke the custom tools built by some other user bases as their tool had the number of columns hardcoded, so adding a column meant they had to modify their tool as well.

Also for some columns complex logic is required to get that column into the report which meant the SP performance degraded, affecting all users - even those who did not need the new column.

We are thinking of various ways to fix this:

1 Default Parameters to control flow

Update the existing SP and control the new functionality by adding a flag as a default parameter to control the code path. By using default parameters, if value of the Parameter is set to true then only call the new functionality. By default it is set to False.

Advantage

  • New Object is not required.
  • On going maintenance is not affected.
  • Testing overhead remains under control.

Disadvantage

  • Since an existing SP is modified, it will need testing of existing functionality as well as new functionality.
  • Since we have no inkling on how the client tools are calling the SPs we can never be sure that we have not broken anything.
  • It will be difficult to handle if same report gets modified again with more requests – will mean more flags and code will become un-readable.

2 New Stored procedure

A new stored procedure will be created for any requirement which changes the signature(Input/Output) of the SP.
The new SP will call the original stored procedure for existing stuff and add the logic for new requirement on top of it.

Advantage

  • Here benefit will be that there will be No impact on the existing procedure hence No Testing required for old logic.

Disadvantage

  • Need to create new objects in database whenever changes are requested. This will be overhead in database maintenance.

Will the execution plan change based on adding a new parameter? If yes then this could adversely affect users who did not request the new column.
Considering a SP is a public interface to the DB and interfaces should be immutable should we go for option 2?
What is the best practice or does it depend on a case by case basis, and what should be the main driving factors when choosing a option?

Thanks in advance!

eastender
  • 434
  • 7
  • 16

7 Answers7

4

Quoting from a disadvantage for your first option:

It will be difficult to handle if same report gets modified again with more requests – will mean more flags and code will become un-readable.

Personally I feel this is the biggest reason not to modify an existing stored procedure to accommodate the new columns.

When bugs come up with a stored procedure that has several branches, it can become very difficult to debug. Also as you hinted at, the execution plan can change with branching/if statements. (sql using different execution plans when running a query and when running that query inside a stored procedure?)

This is very similar to object oriented coding and your instinct is correct that it's best to extend existing objects instead of modify them.

I would go for approach #2. You will have more objects, but at least when an issue comes up, you will be able to know the affected stored procedure has limited scope/impact.

Over time I've learned to grow objects/data structures horizontally, not vertically. In other words, just make something new, don't keep making existing things bigger and bigger and bigger.

Community
  • 1
  • 1
ryan1234
  • 7,237
  • 6
  • 25
  • 36
1

Ok. #2. Definitely. No doubt.

#1 says: "change the existing procedure", causing things to break. No way that's a good thing! Your customers will hate you. Your code just gets more complex meaning it is harder and harder to avoid breaking things leading to more hatred. It will go horribly slowly, and be impossible to tune. And so on.

For #2 you have a stable interface. No hatred. Yay! Seriously, "yay" as in "I still have a job!" as opposed to "boo, I got fired for annoying the hell out of my customers". Seriously. Never ever do #1 for that reason alone. You know this is true. You know it!

Having said that, record what people are doing. Take a user-id as a parameter. Log it. Know your users. Find the ones using old crappy code and ask them nicely to upgrade if necessary.

Your reason given to avoid number 2 is proliferation. But that is only a problem if you don't test stuff. If you do test stuff properly, then proliferation is happening anyway, in your tests. And you can always tune things in #2 if you have to, or at least isolate performance problems.

If the fatter procedure is really great, then retrofit the skinny version with a slimmer version of the fat one. In SQL this is tricky, but copy/paste and cut down your select column list works. Generally I just don't bother to do this. Life is too short. Having really good test code is a much better investment of time, and data schema tend to rarely change in ways that break existing queries.

Okay. Rant over. Serious message. Do #2, or at the very least do NOT do #1 or you will get yourself fired, or hated, or both. I can't think of a better reason than that.

emperorz
  • 429
  • 3
  • 9
1

#2 could be better option than #1 particularly considering the bullet 3 of disadvantages of #1 since requirements keep changing on most of the time. I feel this because disadvantages are dominating here than advantages on either side.

h3xStream
  • 6,293
  • 2
  • 47
  • 57
KKA
  • 161
  • 1
  • 6
1

Easier to go with #2. Nullable SP parameters can create some very difficult to locate bugs. Although, I do employ them from time to time.

Especially when you start getting into joins on nulls and ANSI settings. The way you write the query will change the results dramatically. KISS. (Keep things simple stupid).

Also, if it's a parameterized search for reporting or displaying, I might consider a super-fast fetch of data into a LINQ-able object. Then you can search an in-memory list rather than re-fetching from the database.

Andrew
  • 326
  • 1
  • 7
0

I would also vote for #2. I've seen a few stored procedures which take #1 to the extreme: The SPs has a parameter @Option and a few parameters @param1, @param2, .... The net effect is that this is a single stored procedure that tries to play the role of many stored procedures.

The main disadvantage to #2 is that there are more stored procedures. It may be more difficult to find the one you're looking for, but I think that is a small price to pay for the other advantages you get.

I want to make sure also, that you don't just copy and paste the original stored procedure and add some columns. I've also seen too many of those. If you are only adding a few columns, you can call the original stored procedure and join in the new columns. This will definitely incur a performance penalty if those columns were readily available before, but you won't have to change your original stored procedure (refactoring to allow for good performance and no duplication of the code), nor will you have to maintain two copies of the code (copy and paste for performance).

John Tseng
  • 6,262
  • 2
  • 27
  • 35
0

I am going to suggest a couple of other options based on the options you gave.

Alternative option #1: Add another variable, but instead of making it a default variable base the variable off of customer name. That way Customer A can get his specialized report and Customer B can get his slightly different customized report. This adds a ton of work as updates to the 'Main' portion would have to get copied to all the specialty customer ones. You could do this with branching 'if' statements.

Alternative option #2: Add new stored procedures, just add the customer's name to the stored procedure. Maintenance wise, this might be a little more difficult but it will achieve the same end results, each customer gets his own report type.

orgtigger
  • 3,914
  • 1
  • 20
  • 22
0

Option #2 is the one to choose.

You yourself mentioned (dis)advantages.

While you consider adding new objects to db based on requirement changes, add only necessary objects that don't make your new SP bigger and difficult to maintain.

Nilesh Thakkar
  • 2,877
  • 1
  • 24
  • 43