From adf9419c3ec21f2e3ebc68fad1fd9042983e4e12 Mon Sep 17 00:00:00 2001 From: Todd Martin Date: Sat, 6 Sep 2025 14:06:09 -0400 Subject: [PATCH 1/3] Rearranged SQL docs --- docs/contributing/code-style/sql.md | 656 +++++++++++++++------------- 1 file changed, 345 insertions(+), 311 deletions(-) diff --git a/docs/contributing/code-style/sql.md b/docs/contributing/code-style/sql.md index 209eb6d5f..2fd58af34 100644 --- a/docs/contributing/code-style/sql.md +++ b/docs/contributing/code-style/sql.md @@ -10,12 +10,22 @@ We use the [Repository pattern][repository] with the MSSQL repositories being wr [Dapper][dapper]. Each repository method in turn calls a _Stored Procedure_, which primarily fetches data from _Views_. +## Best practices + +When writing T-SQL code, you should adhere to these general best practices as a guide. More detailed + +1. **Consistency**: Follow established patterns throughout the codebase +2. **Readability**: Prioritize code readability and maintainability +3. **Performance**: Consider index usage and query optimization +4. **Security**: Use parameterized queries and proper data type validation +5. **Modularity**: Break complex operations into smaller, reusable procedures + ## File organization ### Directory structure -- **Schema-based organization**: Files are organized by domain/schema (Auth, Billing, - SecretsManager, Vault, etc.) +- **Schema-based organization**: Files are organized by domain/schema (Auth, Billing, Secrets + Manager, Vault, etc.) - **Object type grouping**: Within each domain, files are grouped by type: - `Tables/` - Table definitions - `Views/` - View definitions @@ -29,17 +39,28 @@ data from _Views_. ### File naming conventions -- **Stored Procedures**: `{EntityName}_{Action}.sql` (e.g., `User_Create.sql`, - `Organization_ReadById.sql`) -- **Tables**: `{EntityName}.sql` (e.g., `User.sql`, `Organization.sql`) -- **Views**: `{EntityName}View.sql` or `{EntityName}{Purpose}View.sql` (e.g., `UserView.sql`, - `ApiKeyDetailsView.sql`) -- **Functions**: `{EntityName}{Purpose}.sql` (e.g., `UserCollectionDetails.sql`) -- **User Defined Types**: `{TypeName}.sql` (e.g., `GuidIdArray.sql`) +- **Stored Procedures**: `{EntityName}_{Action}.sql` + - e.g. `User_Create.sql`, `Organization_ReadById.sql` +- **Tables**: `{EntityName}.sql` + - e.g. `User.sql`, `Organization.sql` +- **Views**: `{EntityName}View.sql` or `{EntityName}{Purpose}View.sql` + - e.g. `UserView.sql`, `ApiKeyDetailsView.sql` +- **Functions**: `{EntityName}{Purpose}.sql` + - e.g. `UserCollectionDetails.sql` +- **User Defined Types**: `{TypeName}.sql` + - e.g. `GuidIdArray.sql` + +:::tip Versioning -## Code formatting standards +When a new version of an entity is introduced and needs to be maintained next to the existing one +during deployment, use versioned names for the different scripts, so that the relationship is +clear - e.g. `_V2` suffix. -### General formatting +::: + +## Code naming and formatting standards + +### General standards - **Indentation**: Use 4 spaces (not tabs) for all SQL code files - **Keywords**: Use UPPERCASE for all SQL keywords (`CREATE`, `SELECT`, `FROM`, `WHERE`, `GROUP BY`, @@ -55,282 +76,6 @@ data from _Views_. - **Commas**: Commas should be placed at the right end of the line - **Parentheses**: Parentheses should be vertically aligned with spanning multiple lines -## Deployment scripts - -There are specific ways deployment scripts should be structured. The goal for these standards is to -ensure that the scripts should be re-runnable. We never intend to run scripts multiple times on an -environment, but the scripts should support it. - -### Tables - -#### Naming conventions - -- **Table Names**: PascalCase (e.g., `[dbo].[User]`, `[dbo].[AuthRequest]`) -- **Column Names**: PascalCase (e.g., `[Id]`, `[CreationDate]`, `[MasterPasswordHash]`) -- **Primary Keys**: `PK_{TableName}` (e.g., `[PK_User]`, `[PK_Organization]`) -- **Foreign Keys**: `FK_{TableName}_{ReferencedTable}` (e.g., `FK_Device_User`) -- **Default Constraints**: `DF_{TableName}_{ColumnName}` (e.g., `[DF_Organization_UseScim]`) - -#### Creating a table - -When creating a table, you must first check if the table exists: - -```sql -IF OBJECT_ID('[dbo].[{table_name}]') IS NULL -BEGIN - CREATE TABLE [dbo].[{table_name}] - ( [Id] UNIQUEIDENTIFIER NOT NULL, - [Column1] DATATYPE NOT NULL, - [Column2] DATATYPE NULL, - [CreationDate] DATETIME2(7) NOT NULL, - [RevisionDate] DATETIME2(7) NOT NULL, - CONSTRAINT [PK_{table_name}] PRIMARY KEY CLUSTERED ([Id] ASC), - CONSTRAINT [FK_{table_name}_{referenced_table}] FOREIGN KEY ([ForeignKeyColumn]) REFERENCES [dbo].[ReferencedTable] ([Id]) - ); -END -GO -``` - -#### Column definition standards - -- **Alignment**: Column names, data types, and nullability vertically aligned using spaces -- **Data Types**: Use consistent type patterns: - - `UNIQUEIDENTIFIER` for IDs - - `DATETIME2(7)` for timestamps - - `NVARCHAR(n)` for Unicode text - - `VARCHAR(n)` for ASCII text - - `BIT` for boolean values - - `TINYINT`, `SMALLINT`, `INT`, `BIGINT` for integers -- **Nullability**: Explicitly specify `NOT NULL` or `NULL` -- **Standard Columns**: Most tables include: - - `[Id] UNIQUEIDENTIFIER NOT NULL` - Primary key - - `[CreationDate] DATETIME2(7) NOT NULL` - Record creation timestamp - - `[RevisionDate] DATETIME2(7) NOT NULL` - Last modification timestamp - -#### Deleting a table - -When deleting a table, use `IF EXISTS` to avoid an error if the table doesn't exist. - -```sql -DROP IF EXISTS [dbo].[{table_name}] -GO -``` - -#### Adding a column to a table - -You must first check to see if the column exists before adding it to the table. - -```sql -IF COL_LENGTH('[dbo].[{table_name}]', '{column_name}') IS NULL -BEGIN - ALTER TABLE [dbo].[{table_name}] - ADD [{column_name}] {DATATYPE} {NULL|NOT NULL}; -END -GO -``` - -When adding a new `NOT NULL` column to an existing table, please re-evaluate the need for it to -truly be required. Do not be afraid of using Nullable\ primitives in C# and in the application -layer, which is almost always going to be better than taking up unnecessary space in the DB per row -with a default value, especially for new functionality or features where it will take a very long -time to be useful for most row-level data, if at all. - -If you do decide to add a `NOT NULL` column, **use a DEFAULT constraint** instead of creating the -column, updating rows and changing the column. This is especially important for the largest tables -like `dbo.User` and `dbo.Cipher`. Our version of SQL Server in Azure uses metadata for default -constraints. This means we can update the default column value **without** updating every row in the -table (which will use a lot of DB I/O). - -This is slow: - -```sql -IF COL_LENGTH('[dbo].[Table]', 'Column') IS NULL -BEGIN - ALTER TABLE - [dbo].[Table] - ADD - [Column] INT NULL -END -GO - -UPDATE - [dbo].[Table] -SET - [Column] = 0 -WHERE - [Column] IS NULL -GO - -ALTER TABLE - [dbo].[Column] -ALTER COLUMN - [Column] INT NOT NULL -GO -``` - -This is better: - -```sql -IF COL_LENGTH('[dbo].[Table]', 'Column' IS NULL -BEGIN - ALTER TABLE - [dbo].[Column] - ADD - [Column] INT NOT NULL CONSTRAINT DF_Table_Column DEFAULT 0 -END -GO -``` - -#### Changing a column data type - -You must wrap the `ALTER TABLE` statement in a conditional block, so that subsequent runs of the -script will not modify the data type again. - -```sql -IF EXISTS ( - SELECT * - FROM INFORMATION_SCHEMA.COLUMNS - WHERE COLUMN_NAME = '{column_name}' AND - DATA_TYPE = '{datatype}' AND - TABLE_NAME = '{table_name}') -BEGIN - ALTER TABLE [dbo].[{table_name}] - ALTER COLUMN [{column_name}] {NEW_TYPE} {NULL|NOT NULL} -END -GO -``` - -#### Adjusting metadata - -When adjusting a table, you should also check to see if that table is referenced in any views. If -the underlying table in a view has been modified, you should run `sp_refreshview` to re-generate the -view metadata. - -```sql -EXECUTE sp_refreshview N'[dbo].[{view_name}]' -GO -``` - -### Views - -#### Naming conventions - -- **View Names**: - - `{EntityName}View` - - Used when the view maps closely to a single table, with little or no joins. (e.g., (e.g., - `[dbo].[ApiKeyView]`) (from ApiKey)) - - `{EntityName}DetailsView` for complex views - - Used for views that combine multiple tables or add logic beyond a basic table select. These - usually serve a specific display or reporting use case and are named to reflect the context - (e.g., `[dbo].[OrganizationUserDetailsView]`) - -- For more complex reads that join multiple tables: - - Create a view with a clear name tied to the main entity: - - `[dbo].[OrganizationUser_MemberAccessDetailsView]` - - Create a stored procedure that reads from it: - - `[dbo].[MemberAccessDetails_ReadByUserId]` - -#### Creating or modifying a view - -We recommend using the `CREATE OR ALTER` syntax for adding or modifying a view. - -```sql -CREATE OR ALTER VIEW [dbo].[{view_name}] -AS -SELECT - * -FROM - [dbo].[{table_name}] -GO -``` - -#### Deleting a view - -When deleting a view, use `IF EXISTS` to avoid an error if the table doesn't exist. - -```sql -DROP IF EXISTS [dbo].[{view_name}] -GO -``` - -#### Adjusting metadata - -When altering views, you may also need to refresh modules (stored procedures or functions) that -reference that view or function so that SQL Server to update its statistics and compiled references -to it. - -```sql -IF OBJECT_ID('[dbo].[{procedure_or_function}]') IS NOT NULL -BEGIN - EXECUTE sp_refreshsqlmodule N'[dbo].[{procedure_or_function}]'; -END -GO -``` - -### Functions and stored procedures - -#### Naming conventions - -- **Stored procedures**: `{EntityName}_{Action}` format (e.g., `[dbo].[User_ReadById]`) - - EntityName: The main table or concept (e.g. User, Organization, Cipher) - - Action: What the procedure does (e.g. Create, ReadById, DeleteMany) -- **Parameters**: Start with `@` and use PascalCase (e.g., `@UserId`, `@OrganizationId`) -- **OUTPUT parameters**: Explicitly declare with `OUTPUT` keyword - -#### Creating or modifying a function or stored procedure - -We recommend using the `CREATE OR ALTER` syntax for adding or modifying a function or stored -procedure. - -```sql -CREATE OR ALTER {PROCEDURE|FUNCTION} [dbo].[{sproc_or_func_name}] -... -GO -``` - -#### Deleting a function or stored procedure - -When deleting a function or stored procedure, use `IF EXISTS` to avoid an error if it doesn't exist. - -```sql -DROP IF EXISTS [dbo].[{sproc_or_func_name}] -GO -``` - -### Creating or modifying an index - -When creating indexes, especially on heavily used tables, our production database can easily become -offline, unusable, hit 100% CPU and many other bad behaviors. Our production database is configured -to do online index builds by default, (so as not to lock the underlying table), so you should -**_not_** specify `ONLINE = ON`, as this may cause failures on some SQL Server editions that do not -support online index rebuilds. Online index creation may cause the index operation to take longer, -but it will not create an underlying schema table lock which prevents all reads and connections to -the table and instead only locks the table of updates during the operation. - -A good example is when creating an index on `dbo.Cipher` or `dbo.OrganizationUser`, those are -heavy-read tables and the locks can cause exceptionally high CPU, wait times and worker exhaustion -in Azure SQL. - -```sql -CREATE NONCLUSTERED INDEX [IX_OrganizationUser_UserIdOrganizationIdStatus] - ON [dbo].[OrganizationUser]([UserId] ASC, [OrganizationId] ASC, [Status] ASC) - INCLUDE ([AccessAll]) -``` - -#### Naming conventions - -- **Indexes**: `IX_{TableName}_{ColumnName(s)}` (e.g., `[IX_User_Email]`) - - The name should clearly indicate the table and the columns being indexed - -#### Index best practices - -- Create indexes after table definition with `GO` separator -- Use descriptive names following `IX_{TableName}_{ColumnName}` pattern -- Include `INCLUDE` clause when beneficial for covering indexes -- Use filtered indexes with `WHERE` clause when appropriate - -## General naming conventions - ### Schema and object prefixes - **Schema**: Use `[dbo]` prefix for all objects @@ -367,6 +112,22 @@ WHERE ### Stored procedures +- **Stored Procedure Name**: `{EntityName}_{Action}` format (e.g., `[dbo].[User_ReadById]`) + - EntityName: The main table or concept (e.g. User, Organization, Cipher) + - Action: What the procedure does (e.g. Create, ReadById, DeleteMany) +- **Parameters**: Start with `@` and use PascalCase (e.g., `@UserId`, `@OrganizationId`) +- **OUTPUT parameters**: Explicitly declare with `OUTPUT` keyword + +:::tip Example of common CRUD operations + +- **Create**: `{EntityName}_Create` procedures +- **Read**: `{EntityName}_ReadById`, `{EntityName}_ReadBy{Criteria}` procedures +- **Read Many**: `{EntityName}_ReadManyByIds`, `{EntityName}_ReadManyBy{Criteria}` procedures +- **Update**: `{EntityName}_Update` procedures +- **Delete**: `{EntityName}_DeleteById`, `{EntityName}_Delete` procedures + +::: + #### Basic structure ```sql @@ -390,8 +151,12 @@ END - Default values on same line as parameter - OUTPUT parameters clearly marked -Note: When adding parameters to an existing stored procedure, a default value must be specified to -ensure backward compatibility and ensure existing code can be called without modification. +:::warning Default parameter values + +When adding parameters to an existing stored procedure, a default value must be specified to ensure +backward compatibility and ensure existing code can be called without modification. + +::: Use `SET NOCOUNT ON` to prevent the automatic return of row count messages, which improves performance and ensures consistent behavior across different client applications that might handle @@ -433,8 +198,59 @@ WHERE [Id] = @Id ``` +### Tables + +- **Table Name**: PascalCase (e.g., [dbo].[User], [dbo].[AuthRequest]) +- **Column Names**: PascalCase (e.g., [Id], [CreationDate], [MasterPasswordHash]) +- **Primary Key**: `PK_{TableName}` (e.g., [PK_User], [PK_Organization]) +- **Foreign Keys**: `FK_{TableName}_{ReferencedTable}` (e.g., FK_Device_User) +- **Default Constraints**: `DF_{TableName}_{ColumnName}` (e.g., [DF_Organization_UseScim]) + +#### Column definition standards + +- **Alignment**: Column names, data types, and nullability vertically aligned using spaces +- **Data Types**: Use consistent type patterns: + - `UNIQUEIDENTIFIER` for IDs + - `DATETIME2(7)` for timestamps + - `NVARCHAR(n)` for Unicode text + - `VARCHAR(n)` for ASCII text + - `BIT` for boolean values + - `TINYINT`, `SMALLINT`, `INT`, `BIGINT` for integers +- **Nullability**: Explicitly specify `NOT NULL` or `NULL` +- **Standard Columns**: Most tables include: + - `[Id] UNIQUEIDENTIFIER NOT NULL` - Primary key + - `[CreationDate] DATETIME2(7) NOT NULL` - Record creation timestamp + - `[RevisionDate] DATETIME2(7) NOT NULL` - Last modification timestamp + +```sql +CREATE TABLE [dbo].[TableName] +( + [Column1] INT IDENTITY(1,1) NOT NULL, + [Column2] NVARCHAR(100) NOT NULL, + [Column3] NVARCHAR(255) NULL, + [Column4] BIT NOT NULL CONSTRAINT [DF_TableName_Column4] DEFAULT (1), + + CONSTRAINT [PK_TableName] PRIMARY KEY CLUSTERED ([Column1] ASC) +); +``` + ### Views +- **View Names**: + - `{EntityName}View` + - Used when the view maps closely to a single table, with little or no joins. (e.g., (e.g., + `[dbo].[ApiKeyView]`) (from ApiKey)) + - `{EntityName}DetailsView` for complex views + - Used for views that combine multiple tables or add logic beyond a basic table select. These + usually serve a specific display or reporting use case and are named to reflect the context + (e.g., `[dbo].[OrganizationUserDetailsView]`) + +- For more complex reads that join multiple tables: + - Create a view with a clear name tied to the main entity: + - `[dbo].[OrganizationUser_MemberAccessDetailsView]` + - Create a stored procedure that reads from it: + - `[dbo].[MemberAccessDetails_ReadByUserId]` + #### Simple views ```sql @@ -470,13 +286,9 @@ WHERE ### Functions -#### Naming conventions - -- **Function names**: `[Schema].[FunctionName]` (e.g., `[dbo].[UserCollectionDetails]`) +- **Function Name**: `[Schema].[FunctionName]` (e.g., `[dbo].[UserCollectionDetails]`) - The name should describe what the function returns -#### Table-valued functions - ```sql CREATE FUNCTION [dbo].[FunctionName](@Parameter DATATYPE) RETURNS TABLE @@ -497,7 +309,7 @@ WHERE ### User defined types -- **Naming**: `[Schema].[TypeName]` (e.g., `[dbo].[GuidIdArray]`) +- **User Defined Type Name**: `[Schema].[TypeName]` (e.g., `[dbo].[GuidIdArray]`) - The name should describe the type ```sql @@ -507,17 +319,18 @@ CREATE TYPE [dbo].[TypeName] AS TABLE ); ``` -## Common patterns +### Indexes -### CRUD operations +- **Index Name**: `IX_{TableName}_{ColumnName(s)}` (e.g., `[IX_User_Email]`) + - The name should clearly indicate the table and the columns being indexed -- **Create**: `{EntityName}_Create` procedures -- **Read**: `{EntityName}_ReadById`, `{EntityName}_ReadBy{Criteria}` procedures -- **Read Many**: `{EntityName}_ReadManyByIds`, `{EntityName}_ReadManyBy{Criteria}` procedures -- **Update**: `{EntityName}_Update` procedures -- **Delete**: `{EntityName}_DeleteById`, `{EntityName}_Delete` procedures +```sql +CREATE NONCLUSTERED INDEX [IX_OrganizationUser_UserIdOrganizationIdStatus] + ON [dbo].[OrganizationUser]([UserId] ASC, [OrganizationId] ASC, [Status] ASC) + INCLUDE ([AccessAll]) +``` -### Error handling +## Error handling - Use `SET NOCOUNT ON` in stored procedures - Implement appropriate transaction handling where needed @@ -565,15 +378,236 @@ END CATCH; - Provide brief explanations for complex CASE statements or calculations - Don't comment unnecessarily, such as commenting that an insert statement is about to be executed -## Best practices +## Deployment scripts -1. **Consistency**: Follow established patterns throughout the codebase -2. **Readability**: Prioritize code readability and maintainability -3. **Performance**: Consider index usage and query optimization -4. **Security**: Use parameterized queries and proper data type validation -5. **Modularity**: Break complex operations into smaller, reusable procedures -6. **Standards**: Always use qualified object names with schema prefix -7. **Versioning**: Use descriptive procedure names for different versions (e.g., `_V2` suffix) +There are specific ways migration scripts should be structured. We do so to adhere to the following +guiding principles: + +✅ **The script must be idempotent**: Always ensure a migration can be run multiple times without +causing errors or duplicating data. We never intend to run scripts multiple times on an environment, +but the scripts must support it. + +✅ **The script must avoid breaking changes**: Migrations should never delete or rename columns that +are still in use by deployed code. + +✅ **The script must maintain schema integrity**: The schema of the database defined in code should +map exactly to the schema of the database in all deployed environments. + +✅ **The script must be backwards compatible**: Code should be able to work with both the old and +new schema during a rolling deployment. See our MSSQL docs for how we put this into practice. + +### Tables + +#### Creating a table + +When creating a table, you must first check if the table exists: + +```sql +IF OBJECT_ID('[dbo].[{table_name}]') IS NULL +BEGIN + CREATE TABLE [dbo].[{table_name}] + ( [Id] UNIQUEIDENTIFIER NOT NULL, + [Column1] DATATYPE NOT NULL, + [Column2] DATATYPE NULL, + [CreationDate] DATETIME2(7) NOT NULL, + [RevisionDate] DATETIME2(7) NOT NULL, + CONSTRAINT [PK_{table_name}] PRIMARY KEY CLUSTERED ([Id] ASC), + CONSTRAINT [FK_{table_name}_{referenced_table}] FOREIGN KEY ([ForeignKeyColumn]) REFERENCES [dbo].[ReferencedTable] ([Id]) + ); +END +GO +``` + +#### Deleting a table + +When deleting a table, use `IF EXISTS` to avoid an error if the table doesn't exist. + +```sql +DROP IF EXISTS [dbo].[{table_name}] +GO +``` + +#### Adding a column to a table + +You must first check to see if the column exists before adding it to the table. + +```sql +IF COL_LENGTH('[dbo].[{table_name}]', '{column_name}') IS NULL +BEGIN + ALTER TABLE [dbo].[{table_name}] + ADD [{column_name}] {DATATYPE} {NULL|NOT NULL}; +END +GO +``` + +When adding a new `NOT NULL` column to an existing table, please re-evaluate the need for it to +truly be required. Do not be afraid of using Nullable\ primitives in C# and in the application +layer, which is almost always going to be better than taking up unnecessary space in the DB per row +with a default value, especially for new functionality or features where it will take a very long +time to be useful for most row-level data, if at all. + +If you do decide to add a `NOT NULL` column, **use a DEFAULT constraint** instead of creating the +column, updating rows and changing the column. This is especially important for the largest tables +like `dbo.User` and `dbo.Cipher`. Our version of SQL Server in Azure uses metadata for default +constraints. This means we can update the default column value **without** updating every row in the +table (which will use a lot of DB I/O). + +This is slow: + +```sql +IF COL_LENGTH('[dbo].[Table]', 'Column') IS NULL +BEGIN + ALTER TABLE + [dbo].[Table] + ADD + [Column] INT NULL +END +GO + +UPDATE + [dbo].[Table] +SET + [Column] = 0 +WHERE + [Column] IS NULL +GO + +ALTER TABLE + [dbo].[Column] +ALTER COLUMN + [Column] INT NOT NULL +GO +``` + +This is better: + +```sql +IF COL_LENGTH('[dbo].[Table]', 'Column' IS NULL +BEGIN + ALTER TABLE + [dbo].[Column] + ADD + [Column] INT NOT NULL CONSTRAINT DF_Table_Column DEFAULT 0 +END +GO +``` + +#### Changing a column data type + +You must wrap the `ALTER TABLE` statement in a conditional block, so that subsequent runs of the +script will not modify the data type again. + +```sql +IF EXISTS ( + SELECT * + FROM INFORMATION_SCHEMA.COLUMNS + WHERE COLUMN_NAME = '{column_name}' AND + DATA_TYPE = '{datatype}' AND + TABLE_NAME = '{table_name}') +BEGIN + ALTER TABLE [dbo].[{table_name}] + ALTER COLUMN [{column_name}] {NEW_TYPE} {NULL|NOT NULL} +END +GO +``` + +#### Adjusting metadata + +When adjusting a table, you should also check to see if that table is referenced in any views. If +the underlying table in a view has been modified, you should run `sp_refreshview` to re-generate the +view metadata. + +```sql +EXECUTE sp_refreshview N'[dbo].[{view_name}]' +GO +``` + +### Views + +#### Creating or modifying a view + +We recommend using the `CREATE OR ALTER` syntax for adding or modifying a view. + +```sql +CREATE OR ALTER VIEW [dbo].[{view_name}] +AS +SELECT + * +FROM + [dbo].[{table_name}] +GO +``` + +#### Deleting a view + +When deleting a view, use `IF EXISTS` to avoid an error if the table doesn't exist. + +```sql +DROP IF EXISTS [dbo].[{view_name}] +GO +``` + +#### Adjusting metadata + +When altering views, you may also need to refresh modules (stored procedures or functions) that +reference that view or function so that SQL Server to update its statistics and compiled references +to it. + +```sql +IF OBJECT_ID('[dbo].[{procedure_or_function}]') IS NOT NULL +BEGIN + EXECUTE sp_refreshsqlmodule N'[dbo].[{procedure_or_function}]'; +END +GO +``` + +### Functions and stored procedures + +#### Creating or modifying a function or stored procedure + +We recommend using the `CREATE OR ALTER` syntax for adding or modifying a function or stored +procedure. + +```sql +CREATE OR ALTER {PROCEDURE|FUNCTION} [dbo].[{sproc_or_func_name}] +... +GO +``` + +#### Deleting a function or stored procedure + +When deleting a function or stored procedure, use `IF EXISTS` to avoid an error if it doesn't exist. + +```sql +DROP IF EXISTS [dbo].[{sproc_or_func_name}] +GO +``` + +### Indices + +When creating indexes, especially on heavily used tables, our production database can easily become +offline, unusable, hit 100% CPU and many other bad behaviors. Our production database is configured +to do online index builds by default, (so as not to lock the underlying table), so you should +**_not_** specify `ONLINE = ON`, as this may cause failures on some SQL Server editions that do not +support online index rebuilds. Online index creation may cause the index operation to take longer, +but it will not create an underlying schema table lock which prevents all reads and connections to +the table and instead only locks the table of updates during the operation. + +A good example is when creating an index on `dbo.Cipher` or `dbo.OrganizationUser`, those are +heavy-read tables and the locks can cause exceptionally high CPU, wait times and worker exhaustion +in Azure SQL. + +```sql +CREATE NONCLUSTERED INDEX [IX_OrganizationUser_UserIdOrganizationIdStatus] + ON [dbo].[OrganizationUser]([UserId] ASC, [OrganizationId] ASC, [Status] ASC) + INCLUDE ([AccessAll]) +``` + +#### Index best practices + +- Create indexes after table definition with `GO` separator +- Include `INCLUDE` clause when beneficial for covering indexes +- Use filtered indexes with `WHERE` clause when appropriate [repository]: https://docs.microsoft.com/en-us/dotnet/architecture/microservices/microservice-ddd-cqrs-patterns/infrastructure-persistence-layer-design From 3760119fa4a531ca38d02aeb0a384194a43e59fb Mon Sep 17 00:00:00 2001 From: Todd Martin Date: Sat, 6 Sep 2025 16:09:22 -0400 Subject: [PATCH 2/3] More changes --- docs/contributing/code-style/sql.md | 45 ++++++++++++----------------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/docs/contributing/code-style/sql.md b/docs/contributing/code-style/sql.md index 2fd58af34..3e6064a52 100644 --- a/docs/contributing/code-style/sql.md +++ b/docs/contributing/code-style/sql.md @@ -12,8 +12,6 @@ data from _Views_. ## Best practices -When writing T-SQL code, you should adhere to these general best practices as a guide. More detailed - 1. **Consistency**: Follow established patterns throughout the codebase 2. **Readability**: Prioritize code readability and maintainability 3. **Performance**: Consider index usage and query optimization @@ -58,14 +56,17 @@ clear - e.g. `_V2` suffix. ::: -## Code naming and formatting standards +## Code style ### General standards +These standards should be applied across any T-SQL scripts that you write. + - **Indentation**: Use 4 spaces (not tabs) for all SQL code files - **Keywords**: Use UPPERCASE for all SQL keywords (`CREATE`, `SELECT`, `FROM`, `WHERE`, `GROUP BY`, `ORDER BY`, `JOIN`, `ON`, `INTO`, `TOP`, etc.) - **Object names**: Always use square brackets `[dbo].[TableName]` +- **Schema**: Use `[dbo]` prefix for all objects - **Line endings**: Use consistent line breaks with proper indentation - **Trailing spaces**: Should be trimmed from the end of lines. Use `[ \t]+$` as a regex find/replace @@ -74,19 +75,14 @@ clear - e.g. `_V2` suffix. makes code changes easily detectable - **Blank lines**: Separate sections of code with at least one blank line - **Commas**: Commas should be placed at the right end of the line -- **Parentheses**: Parentheses should be vertically aligned with spanning multiple lines +- **Parentheses**: Parentheses should be vertically aligned with spanning multiple line -### Schema and object prefixes - -- **Schema**: Use `[dbo]` prefix for all objects -- **Object names**: Always use square brackets `[dbo].[TableName]` - -### Select statements +### `SELECT` statements - `SELECT` keyword on its own line - Column names indented (4 spaces) - One column per line for multi-column selects -- Callout the specific table/alias for where a column is from when joining to other tables +- Call out the specific table/alias for where a column is from when joining to other tables - `FROM` keyword on separate line, aligned with `SELECT` - `FROM` clause indented (4 spaces) - Use aliases for table names when joining to other tables @@ -149,7 +145,7 @@ END - One parameter per line - Align parameters with consistent indentation (4 spaces) - Default values on same line as parameter -- OUTPUT parameters clearly marked +- `OUTPUT` parameters clearly marked :::warning Default parameter values @@ -162,10 +158,10 @@ Use `SET NOCOUNT ON` to prevent the automatic return of row count messages, whic performance and ensures consistent behavior across different client applications that might handle these messages differently. -#### Insert statements +#### `INSERT` statements - Column list in parentheses, one column per line -- VALUES clause with parameters aligned +- `VALUES` clause with parameters aligned - Proper indentation for readability ```sql @@ -181,7 +177,7 @@ VALUES ) ``` -#### Update statements +#### `UPDATE` statements - `UPDATE` and table name on different lines - `SET` clause with each column assignment on separate line @@ -206,7 +202,7 @@ WHERE - **Foreign Keys**: `FK_{TableName}_{ReferencedTable}` (e.g., FK_Device_User) - **Default Constraints**: `DF_{TableName}_{ColumnName}` (e.g., [DF_Organization_UseScim]) -#### Column definition standards +#### Column definitions - **Alignment**: Column names, data types, and nullability vertically aligned using spaces - **Data Types**: Use consistent type patterns: @@ -236,20 +232,17 @@ CREATE TABLE [dbo].[TableName] ### Views -- **View Names**: +- **View Name**: - `{EntityName}View` - - Used when the view maps closely to a single table, with little or no joins. (e.g., (e.g., - `[dbo].[ApiKeyView]`) (from ApiKey)) + - Used when the view maps closely to a single table, with little or no joins. (e.g., + `[dbo].[ApiKeyView]` from `ApiKey`) - `{EntityName}DetailsView` for complex views - Used for views that combine multiple tables or add logic beyond a basic table select. These usually serve a specific display or reporting use case and are named to reflect the context (e.g., `[dbo].[OrganizationUserDetailsView]`) - -- For more complex reads that join multiple tables: - - Create a view with a clear name tied to the main entity: - - `[dbo].[OrganizationUser_MemberAccessDetailsView]` - - Create a stored procedure that reads from it: - - `[dbo].[MemberAccessDetails_ReadByUserId]` + - For views that combine entities, create a view with a clear name tied to the main entity (e.g., + `[dbo].[OrganizationUser_MemberAccessDetailsView]`) and a stored procedure that reads from it + (e.g., `[dbo].[MemberAccessDetails_ReadByUserId]`). #### Simple views @@ -583,7 +576,7 @@ DROP IF EXISTS [dbo].[{sproc_or_func_name}] GO ``` -### Indices +### Indexes When creating indexes, especially on heavily used tables, our production database can easily become offline, unusable, hit 100% CPU and many other bad behaviors. Our production database is configured From 6999edb65eabe46cba6ab65a08c2ceb9f6a30bf1 Mon Sep 17 00:00:00 2001 From: Todd Martin Date: Tue, 9 Sep 2025 18:28:15 -0400 Subject: [PATCH 3/3] PR feedback --- docs/contributing/code-style/sql.md | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/docs/contributing/code-style/sql.md b/docs/contributing/code-style/sql.md index 3e6064a52..d4a02669c 100644 --- a/docs/contributing/code-style/sql.md +++ b/docs/contributing/code-style/sql.md @@ -52,7 +52,7 @@ data from _Views_. When a new version of an entity is introduced and needs to be maintained next to the existing one during deployment, use versioned names for the different scripts, so that the relationship is -clear - e.g. `_V2` suffix. +clear - e.g. a `_V2` suffix. ::: @@ -68,14 +68,12 @@ These standards should be applied across any T-SQL scripts that you write. - **Object names**: Always use square brackets `[dbo].[TableName]` - **Schema**: Use `[dbo]` prefix for all objects - **Line endings**: Use consistent line breaks with proper indentation -- **Trailing spaces**: Should be trimmed from the end of lines. Use `[ \t]+$` as a regex - find/replace - **Vertical lists**: Vertically list items as much as feasibly possible, and use consistent indentation to make vertical listing quick and easy. A vertical list is much easier to compare and makes code changes easily detectable - **Blank lines**: Separate sections of code with at least one blank line - **Commas**: Commas should be placed at the right end of the line -- **Parentheses**: Parentheses should be vertically aligned with spanning multiple line +- **Parentheses**: Parentheses should be vertically aligned with spanning multiple lines ### `SELECT` statements @@ -376,18 +374,17 @@ END CATCH; There are specific ways migration scripts should be structured. We do so to adhere to the following guiding principles: -✅ **The script must be idempotent**: Always ensure a migration can be run multiple times without -causing errors or duplicating data. We never intend to run scripts multiple times on an environment, -but the scripts must support it. +- **It must be idempotent**: Always ensure a migration can be run multiple times without causing + errors or duplicating data. -✅ **The script must avoid breaking changes**: Migrations should never delete or rename columns that -are still in use by deployed code. +- **It must avoid breaking changes**: Migrations should never delete or rename columns that are + still in use by deployed code. -✅ **The script must maintain schema integrity**: The schema of the database defined in code should -map exactly to the schema of the database in all deployed environments. +- **It must maintain schema integrity**: The schema of the database defined in code should map + exactly to the schema of the database in all deployed environments. -✅ **The script must be backwards compatible**: Code should be able to work with both the old and -new schema during a rolling deployment. See our MSSQL docs for how we put this into practice. +- **It must be backwards compatible**: Code should be able to work with both the old and new schema + during a rolling deployment. ### Tables