Stored Procedure Code Review Checklist

In any enterprise environment developing a database system includes the database code review. We have to document and follow the best practices in designing and developing database code. Most of the time while interacting with the customers we may need to answer the question “What you consider in reviewing the code?” Here I am going to showcase stored procedure code review checklist that we use for reviewing or unit testing a stored procedure.

Stored Procedure Development Life Cycle:

First let’s see what are the steps followed in developing a stored procedure:

• Get the requirement – all required inputs and expected outcome
• Write the test cases to validate the business requirement
• Design and create the code / stored procedure
• Debug, Compile and Run the procedure
• Cross check all test cases passed
• Send it for code review
• Apply suggested changes if any from code review
• Deploy it to the intended environment
• Document the process

Why Best Practices / Code Review?

In enterprise environment we do follow a best practices guide for developing and reviewing database code. Now we’ll see what are the advantages in following the best practices guide / reviewing the code?

Dependency:

It minimize the resource dependency as all database objects follow a specific standard. One can easily understand and deal with the code changes.

Integration:

Easily integrated with the existing environment

Optimization:

We can see the best optimized and error free code

Stored Procedure Code Review Check List

We have defined checklist in category wise. Below are the various categories in stored procedure code review check list.

• General Standards
• Scalability
• Security
• Transactions
• Performance
• Functionality
• Environment Based Settings

General Standards: (Code Format, Naming Conventions, Datatype and Data Length, Syntax):

• Always follow a template in designing stored procedure so that it can easier developer job while designing and integrating. For example each stored procedure should be defined as various blocks such as “Comments Section”, “Variable Declaration”, “Actual Body”, “Audit”, ”Exception Handling”, “Temp_Obj_Removel” and define environment sections if any required.
• Check proper comments are used or not. Always describe procedure, inputs and expected output in comments section.
• Check naming conventions are used properly for procedure name, variables and other internal objects.
• Check all objects used inside the procedure are prefixed with the schema name and column names are referencing with table alias.
• Check all table columns used / mapped are using the correct datatypes and column length.
• Check if all required SET based options enabled are not.
• Check if there are any temporary objects (Temporary tables, cursors etc) used, if yes make sure these objects closed / removed once their usage is done.
• Make sure Errors are handling properly.
• Define NULL acceptance in the procedure and code accordingly.
• Lining up parameter names, data types, and default values.
• Check spaces and line breaks are using properly.
• Check BEGIN / END are using properly.
• Check parentheses are using properly around AND / OR blocks.

Scalability:

• Use fully qualified names (Ex: Instead of PROC use PROCEDURE) for a better integration.
• Check if we are using any deprecated features.
• Check if any other/nested dependent objects used and make sure that all objects are available in DB and all functioning properly and add them into dependent list.
• Never use “SELECT *” instead use all required columns.
• If there are any triggers defined on tables used inside the procedure, make sure these triggers are working as expected.

Security:

• In case of any errors make sure that the complete error information is not throwing to the application instead use a centralized table to hold the error information and send a custom error message to the application.
• Apply encryption procedures while dealing with sensitive information (Ex: Credit Card numbers, pass codes etc.).
• If any dynamic SQL is used make sure it executes through only SP_EXECUTESQL only.
• Prefer views instead of tables wherever is possible.

• Document all permissions required to run the procedure.

Transactions:

• If any transactions are used, check it is following ACID properties as per the business requirement.
• Keep the transaction length as short as possible and do not select data within the transaction rather than select required data before starting the transaction and process it inside the transaction.
• Check commit and ROLLBACK is available happening as expected, cross check when using nested stored procedures.
• Avoid transactions that require user input to commit.

Performance:

• Cross check we are selecting / retrieving only required data throughout the procedure, always use Column names instead of “SELECT *”.
• Check the column order in where clause, we should remember it impact the index usage, change the order if required.
• Avoid using functions / conversions while selecting and comparing, If result set is having 30 rows means that function is called >=30 times. let’s say “WHERE <TAB.ColName> = MONTH (@DateParam)”, we can fulfill this by creating a local variable and assigning this value to that and we can use that variable in where clause.
• Cross check if we can have a better / short way to get the same outcome with fewer joins.
• Always do filter data as much as we can and then apply required operations.
• Have a look on aggregations if any. Always do aggregations on a possible shortest dataset. Example we have a requirement “We want to know the top selling product details on each eStore”. Now do not join Product_Orders, Product_Details and group by on e-store name by selecting max of revenue e-store wise. Instead of doing this first get the productID’s with highest income e-Store wise and then map it with Product_Details.
• Check if there is any chance for bad parameter sniffing: Make sure that procedure parameters are assigning to local variables and referring these variables in queries instead of directly referring PROCEDURE parameters.
• Choose the best temporary object (Temp_Table, Table_Variable, CTE and Derived_Table) based on the requirement, here we should predict the near future.
• Try to use TRUNCATE instead of DELETE whenever is possible, remember we should know the difference and the impact.
• Check the response / execution time and make sure it is under the benchmark.
• Avoid cursors, use while loop instead.
• Check for the alternatives for costly operators such as NOT LIKE.

• Make sure that it returns only the required rows and columns.
• Analyze cost based execution plan to make sure No Bookmark / RID Lookup, No Table/Index Scans taking more cost, No Sort – Check if we can use Order By, Check Estimated and Actual counts.
• Have a look at Optimizer Overrides – Review the code to determine if index hints or NOLOCK clauses are really necessary. These hints could be beneficial in the short term, but these overrides might impact negatively when data changes happen or database migrated to new version.

Functionality:

• Prepare various test cases and make sure all test cases works fine. Example prepare test case to send all possible inputs to a PROCEDURE, define the expected output and compare with the actual output.
• Check Code is error free and parsing correctly and executing without any issue.
• Check output result set is coming properly, number of rows, number of columns, column names, datatypes etc.

Environment Based Settings:

• Document all environments based settings and follow those instructions in designing the procedure.

Ex 1: In a highly secure database environment, all procedures should call a nested Audit procedure which collects all session details and stored in an audit table.

Ex 2: In an environment before performing and bulk operation we have to get the latest lookup values in lookup tables.

Summary:

• Always define your own standards and follow best practices in designing database code.
• Prepare an excel sheet with the required checklist and make sure that the sql developer is filling the sheet before sending it to the code review.
• Initially it might take some extra time in development phase but it simplifies the code review process and leads to the best productivity.