Add bulk Delete and Update methods using SqlExpression by pvictorlv · Pull Request #317 · henkmollema/Dommel

pvictorlv · 2025-11-29T20:07:35Z

Introduces new Delete and Update methods to DommelMapper that accept a SqlExpression builder for bulk operations. SqlExpression now supports building SET clauses for updates.

Copilot

Pull request overview

This PR introduces bulk update and delete operations to DommelMapper using the SqlExpression builder pattern. It adds new overloads for Update/UpdateAsync and Delete/DeleteAsync methods that accept Action callbacks to build SQL expressions, enabling flexible bulk operations with WHERE clauses. The SqlExpression class is enhanced with a new Set() method to specify column values for updates.

Key changes:

Added Set() method to SqlExpression for building UPDATE SET clauses
Added bulk Update/UpdateAsync methods accepting SqlExpression builders
Added bulk Delete/DeleteAsync methods accepting SqlExpression builders

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.

File	Description
src/Dommel/SqlExpression.cs	Added `Set()` method for UPDATE operations and enhanced `ToSql()` to build SET clauses; changed `AddParameter` signature to accept nullable values
src/Dommel/Update.cs	Added new `Update` and `UpdateAsync` overloads that accept SqlExpression builders for bulk update operations
src/Dommel/Delete.cs	Added new `Delete` and `DeleteAsync` overloads that accept SqlExpression builders for bulk delete operations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-29T20:13:13Z

+    public static int Delete<TEntity>(this IDbConnection connection, Action<SqlExpression<TEntity>> sqlBuilder, IDbTransaction? transaction = null)
+    {
+        var builder = GetSqlBuilder(connection);
+        var expression = CreateSqlExpression<TEntity>(builder);
+        sqlBuilder(expression);
+        var sql = expression.ToSql(out var parameters);
+        var table = Resolvers.Table(typeof(TEntity), builder);
+        sql = $"delete from {table}{sql}";
+        LogQuery<TEntity>(sql);
+        return connection.Execute(sql, parameters, transaction);
+    }


The new bulk Delete method with SqlExpression builder lacks test coverage. Based on the test patterns in this codebase (e.g., DeleteTests.cs with DeleteMultiple), integration tests should be added to verify:

Basic delete with Where clause

Delete with complex Where conditions

Verification that entities are actually deleted

Example test structure:

[Theory] [ClassData(typeof(DatabaseTestData))] public void DeleteWithSqlExpression(DatabaseDriver database) { using var con = database.GetConnection(); var countBefore = con.Count<Product>(p => p.CategoryId == 1); var affected = con.Delete<Product>(sql => sql.Where(p => p.CategoryId == 1)); Assert.Equal(countBefore, affected); Assert.Equal(0, con.Count<Product>(p => p.CategoryId == 1)); }

Copilot · 2025-11-29T20:13:13Z

+    public static async Task<int> DeleteAsync<TEntity>(this IDbConnection connection, Action<SqlExpression<TEntity>> sqlBuilder, IDbTransaction? transaction = null, CancellationToken cancellationToken = default)
+    {
+        var builder = GetSqlBuilder(connection);
+        var expression = CreateSqlExpression<TEntity>(builder);
+        sqlBuilder(expression);
+        var sql = expression.ToSql(out var parameters);
+        var table = Resolvers.Table(typeof(TEntity), builder);
+        sql = $"delete from {table}{sql}";
+        LogQuery<TEntity>(sql);
+        return await connection.ExecuteAsync(new CommandDefinition(sql, parameters, transaction: transaction, cancellationToken: cancellationToken));
+    }


The new async bulk DeleteAsync method with SqlExpression builder lacks test coverage. Based on the test patterns in this codebase (e.g., DeleteTests.cs with DeleteMultipleAsync), integration tests should be added similar to the synchronous version:

[Theory] [ClassData(typeof(DatabaseTestData))] public async Task DeleteAsyncWithSqlExpression(DatabaseDriver database) { using var con = database.GetConnection(); var countBefore = await con.CountAsync<Product>(p => p.CategoryId == 1); var affected = await con.DeleteAsync<Product>(sql => sql.Where(p => p.CategoryId == 1)); Assert.Equal(countBefore, affected); Assert.Equal(0, await con.CountAsync<Product>(p => p.CategoryId == 1)); }

Copilot · 2025-11-29T20:13:13Z

+    /// <summary>
+    /// Updates the specified column with the specified value.
+    /// </summary>
+    /// <typeparam name="TValue">The type of the value.</typeparam>
+    /// <param name="selector">The column to update.</param>
+    /// <param name="value">The value to set.</param>
+    /// <returns>The current <see cref="SqlExpression{TEntity}"/> instance.</returns>


[nitpick] The documentation for the Set method could be improved to include a usage example, as this is a new API feature. Following the pattern of other SqlExpression methods, consider adding an example:

/// <summary> /// Updates the specified column with the specified value. /// </summary> /// <example> /// <code> /// connection.Update<Product>(sql => /// sql.Set(p => p.Name, "Updated") /// .Where(p => p.CategoryId == 1)); /// </code> /// </example> /// <typeparam name="TValue">The type of the value.</typeparam> /// <param name="selector">The column to update.</param> /// <param name="value">The value to set.</param> /// <returns>The current <see cref="SqlExpression{TEntity}"/> instance.</returns>

Copilot · 2025-11-29T20:13:13Z

+    /// <typeparam name="TEntity">The type of the entity.</typeparam>
+    /// <param name="connection">The connection to the database. This can either be open or closed.</param>
+    /// <param name="sqlBuilder">A callback to build a <see cref="SqlExpression{TEntity}"/>.</param>
+    /// <param name="transaction">Optional transaction for the command.</param>
+    /// <returns>The number of rows affected.</returns>
+    public static int Update<TEntity>(this IDbConnection connection, Action<SqlExpression<TEntity>> sqlBuilder, IDbTransaction? transaction = null)


[nitpick] The documentation for this new bulk Update method could be improved with a usage example to clarify how it differs from the single-entity Update method and to show the Set() method usage:

/// <summary> /// Updates the entities matching the specified predicate with the specified values. /// </summary> /// <example> /// <code> /// var affected = connection.Update<Product>(sql => /// sql.Set(p => p.Name, "Updated") /// .Set(p => p.Price, 9.99m) /// .Where(p => p.CategoryId == 1)); /// </code> /// </example> /// <typeparam name="TEntity">The type of the entity.</typeparam> /// <param name="connection">The connection to the database. This can either be open or closed.</param> /// <param name="sqlBuilder">A callback to build a <see cref="SqlExpression{TEntity}"/>.</param> /// <param name="transaction">Optional transaction for the command.</param> /// <returns>The number of rows affected.</returns>

Suggested change

/// <typeparam name="TEntity">The type of the entity.</typeparam>

/// <param name="connection">The connection to the database. This can either be open or closed.</param>

/// <param name="sqlBuilder">A callback to build a <see cref="SqlExpression{TEntity}"/>.</param>

/// <param name="transaction">Optional transaction for the command.</param>

/// <returns>The number of rows affected.</returns>

public static int Update<TEntity>(this IDbConnection connection, Action<SqlExpression<TEntity>> sqlBuilder, IDbTransaction? transaction = null)

/// <example>

/// <code>

/// // Bulk update example: update all products in category 1

/// var affected = connection.Update<Product>(sql =>

/// sql.Set(p => p.Name, "Updated")

/// .Set(p => p.Price, 9.99m)

/// .Where(p => p.CategoryId == 1));

/// </code>

/// </example>

/// <typeparam name="TEntity">The type of the entity.</typeparam>

/// <param name="connection">The connection to the database. This can either be open or closed.</param>

/// <param name="sqlBuilder">A callback to build a <see cref="SqlExpression{TEntity}"/>.</param>

/// <param name="transaction">Optional transaction for the command.</param>

/// <returns>The number of rows affected.</returns>

Copilot · 2025-11-29T20:13:14Z

+    /// <typeparam name="TEntity">The type of the entity.</typeparam>
+    /// <param name="connection">The connection to the database. This can either be open or closed.</param>
+    /// <param name="sqlBuilder">A callback to build a <see cref="SqlExpression{TEntity}"/>.</param>
+    /// <param name="transaction">Optional transaction for the command.</param>
+    /// <returns>The number of rows affected.</returns>
+    public static int Delete<TEntity>(this IDbConnection connection, Action<SqlExpression<TEntity>> sqlBuilder, IDbTransaction? transaction = null)


[nitpick] The documentation for this new bulk Delete method could be improved with a usage example to clarify how it differs from the single-entity Delete method and existing DeleteMultiple:

/// <summary> /// Deletes the entities matching the specified predicate. /// </summary> /// <example> /// <code> /// var affected = connection.Delete<Product>(sql => /// sql.Where(p => p.CategoryId == 1)); /// </code> /// </example> /// <typeparam name="TEntity">The type of the entity.</typeparam> /// <param name="connection">The connection to the database. This can either be open or closed.</param> /// <param name="sqlBuilder">A callback to build a <see cref="SqlExpression{TEntity}"/>.</param> /// <param name="transaction">Optional transaction for the command.</param> /// <returns>The number of rows affected.</returns>

Suggested change

/// <typeparam name="TEntity">The type of the entity.</typeparam>

/// <param name="connection">The connection to the database. This can either be open or closed.</param>

/// <param name="sqlBuilder">A callback to build a <see cref="SqlExpression{TEntity}"/>.</param>

/// <param name="transaction">Optional transaction for the command.</param>

/// <returns>The number of rows affected.</returns>

public static int Delete<TEntity>(this IDbConnection connection, Action<SqlExpression<TEntity>> sqlBuilder, IDbTransaction? transaction = null)

/// <example>

/// <code>

/// // Bulk delete: removes all products in category 1.

/// var affected = connection.Delete<Product>(sql => sql.Where(p => p.CategoryId == 1));

///

/// // Single-entity delete: removes a specific product by its primary key.

/// var affected = connection.Delete(product);

///

/// // DeleteMultiple: removes a collection of products by their primary keys.

/// var affected = connection.DeleteMultiple(products);

/// </code>

/// </example>

/// <typeparam name="TEntity">The type of the entity.</typeparam>

/// <param name="connection">The connection to the database. This can either be open or closed.</param>

/// <param name="sqlBuilder">A callback to build a <see cref="SqlExpression{TEntity}"/>.</param>

/// <param name="transaction">Optional transaction for the command.</param>

/// <returns>The number of rows affected.</returns>

Copilot · 2025-11-29T20:13:14Z

+    public static int Update<TEntity>(this IDbConnection connection, Action<SqlExpression<TEntity>> sqlBuilder, IDbTransaction? transaction = null)
+    {
+        var builder = GetSqlBuilder(connection);
+        var expression = CreateSqlExpression<TEntity>(builder);
+        sqlBuilder(expression);
+        var sql = expression.ToSql(out var parameters);
+        var table = Resolvers.Table(typeof(TEntity), builder);
+        sql = $"update {table}{sql}";
+        LogQuery<TEntity>(sql);
+        return connection.Execute(sql, parameters, transaction);
+    }


The new bulk Update method with SqlExpression builder lacks test coverage. Based on the test patterns in this codebase (e.g., FromTests.cs), integration tests should be added to verify:

Basic update with Set and Where clauses

Update with multiple Set calls

Update without Where clause (all rows)

Update with complex Where conditions

Verification that updated values persist

Example test structure:

[Theory] [ClassData(typeof(DatabaseTestData))] public void UpdateWithSqlExpression(DatabaseDriver database) { using var con = database.GetConnection(); var affected = con.Update<Product>(sql => sql.Set(p => p.Name, "Updated") .Where(p => p.CategoryId == 1)); Assert.True(affected > 0); var updated = con.Select<Product>(p => p.CategoryId == 1); Assert.All(updated, p => Assert.Equal("Updated", p.Name)); }

Copilot · 2025-11-29T20:13:15Z

+        if (_setStatements.Count > 0)
+        {
+            var setBuilder = new StringBuilder();
+            foreach (var (column, paramName) in _setStatements)
+            {
+                if (setBuilder.Length == 0)
+                {
+                    setBuilder.Append(" set ");
+                }
+                else
+                {
+                    setBuilder.Append(", ");
+                }
+                setBuilder.Append($"{column} = {paramName}");
+            }
+            query += setBuilder.ToString();
+        }


The ToSql() method generates SQL by concatenating SELECT, SET, and WHERE clauses in sequence. If both _selectQuery and _setStatements are populated (e.g., by calling both Select() and Set() on the same expression), this will produce invalid SQL like select * from table set column = value where ....

While the current Update/Delete methods don't call Select(), this creates a footgun in the API. Consider adding validation:

public string ToSql() { var query = ""; if (!string.IsNullOrEmpty(_selectQuery)) { query += _selectQuery; } if (_setStatements.Count > 0) { if (!string.IsNullOrEmpty(_selectQuery)) { throw new InvalidOperationException("Cannot use both Select() and Set() in the same expression."); } // ... rest of SET logic } // ... }

Copilot · 2025-11-29T20:13:15Z

+        AddParameter(value, out var paramName);
+        _setStatements.Add((column!, paramName));


The VisitExpression method can return non-string values (e.g., Expression objects when the expression type is not recognized). Using the null-forgiving operator ! on line 120 after a potentially null cast is unsafe and could lead to a NullReferenceException at runtime.

Consider adding validation to ensure the column name is valid:

var column = VisitExpression(selector.Body) as string; if (string.IsNullOrEmpty(column)) { throw new ArgumentException($"Could not resolve column from expression.", nameof(selector)); } AddParameter(value, out var paramName); _setStatements.Add((column, paramName));

Suggested change

AddParameter(value, out var paramName);

_setStatements.Add((column!, paramName));

if (string.IsNullOrEmpty(column))

{

throw new ArgumentException($"Could not resolve column from expression.", nameof(selector));

}

AddParameter(value, out var paramName);

_setStatements.Add((column, paramName));

Copilot · 2025-11-29T20:13:15Z

+    public static async Task<int> UpdateAsync<TEntity>(this IDbConnection connection, Action<SqlExpression<TEntity>> sqlBuilder, IDbTransaction? transaction = null, CancellationToken cancellationToken = default)
+    {
+        var builder = GetSqlBuilder(connection);
+        var expression = CreateSqlExpression<TEntity>(builder);
+        sqlBuilder(expression);
+        var sql = expression.ToSql(out var parameters);
+        var table = Resolvers.Table(typeof(TEntity), builder);
+        sql = $"update {table}{sql}";
+        LogQuery<TEntity>(sql);
+        return await connection.ExecuteAsync(new CommandDefinition(sql, parameters, transaction: transaction, cancellationToken: cancellationToken));
+    }


The new async bulk UpdateAsync method with SqlExpression builder lacks test coverage. Based on the test patterns in this codebase (e.g., FromTests.cs), integration tests should be added similar to the synchronous version:

[Theory] [ClassData(typeof(DatabaseTestData))] public async Task UpdateAsyncWithSqlExpression(DatabaseDriver database) { using var con = database.GetConnection(); var affected = await con.UpdateAsync<Product>(sql => sql.Set(p => p.Name, "Updated") .Where(p => p.CategoryId == 1)); Assert.True(affected > 0); var updated = await con.SelectAsync<Product>(p => p.CategoryId == 1); Assert.All(updated, p => Assert.Equal("Updated", p.Name)); }

Copilot · 2025-11-29T20:13:15Z

+                }
+                setBuilder.Append($"{column} = {paramName}");
+            }
+            query += setBuilder.ToString();


Redundant call to 'ToString' on a String object.

Fixed some issues addressed by github copilot

sonarqubecloud · 2026-04-28T10:13:45Z

Quality Gate failed

Failed conditions
4 Security Hotspots

See analysis details on SonarQube Cloud

Add bulk Delete and Update methods using SqlExpression

a968c2d

Introduces new Delete and Update methods to DommelMapper that accept a SqlExpression builder for bulk operations. SqlExpression now supports building SET clauses for updates.

Copilot AI review requested due to automatic review settings November 29, 2025 20:07

pvictorlv marked this pull request as draft November 29, 2025 20:07

Copilot started reviewing on behalf of pvictorlv November 29, 2025 20:08 View session

Copilot finished reviewing on behalf of pvictorlv November 29, 2025 20:12

Copilot AI reviewed Nov 29, 2025

View reviewed changes

pvictorlv added 2 commits November 30, 2025 18:26

tests

cf0f5f8

Copilot review fix

e1ee302

Fixed some issues addressed by github copilot

pvictorlv marked this pull request as ready for review December 9, 2025 17:45

Navigation Menu

Uh oh!

Add bulk Delete and Update methods using SqlExpression - #317

Add bulk Delete and Update methods using SqlExpression#317
pvictorlv wants to merge 3 commits into
henkmollema:masterfrom
pvictorlv:bulk-update-delete

pvictorlv commented Nov 29, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

Copilot AI Nov 29, 2025

Uh oh!

sonarqubecloud Bot commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		AddParameter(value, out var paramName);
		_setStatements.Add((column!, paramName));

Uh oh!

Conversation

pvictorlv commented Nov 29, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Apr 28, 2026

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants