Skip to main content

SQL Code Review-Tips

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.

Comments