Repository navigation
fix: stop silent data loss and wrong-SQL edge cases in bulk operations - #34
Merged
Merged
Conversation
Library
- Reject entities with a shadow property (NotSupportedException). Values are read
from CLR properties by reflection; a shadow FK / TPH discriminator has none and
was sent as NULL without warning.
- Read option expressions from the expression tree. `x => x.CreatedAt` used to be
compiled, run on a blank instance, and have DateTime's own properties read back,
so IgnoreOnInsert/IgnoreOnUpdate/UniqueKeys silently did nothing. Only
`x => x.Prop` and `x => new { x.A, x.B }` are accepted; anything else throws.
- Fall back to the primary key when the model has no unique index; only a
keyless entity throws MissingPrimaryKeyException. Key resolution is now one
method shared by update and delete, and a UniqueKeys name that is not a mapped
property is an ArgumentException instead of a silent fallback.
- Keep key columns in the derived table when IgnoreOnUpdate names them; the JOIN
referenced tmp.`Id` that the SELECT no longer produced.
- Update with nothing to SET throws instead of emitting `SET ` on its own; merge
of a key-only entity assigns a key to itself, MySQL's insert-if-absent idiom.
- Close the connection only if this call opened it. One the caller opened (to
share across calls, or enlisted in a TransactionScope) was closed under them.
- Join Database.CurrentTransaction when the context already has one open instead
of failing with "Transactions may not be nested".
- Name the actual type in "Unable to resolve EntityType" (was the literal 'T').
Dependencies
- EF Core 8.0.4 -> 8.0.30 removes Microsoft.Extensions.Caching.Memory 8.0.0
(GHSA-qj66-m88j-hmgj) from the shipped package; Pomelo 8.0.2 -> 8.0.3.
- Testcontainers.MySql 3.8.0 -> 4.14.0 drops SSH.NET 2023.0.0 (GHSA-q939-rpr3-3284)
from the test project. Its own wait strategy replaces the port check.
- Drop the unused EntityFrameworkCore.InMemory reference.
CI / repo
- sonarcloud.yml: pass the token through the env var already declared instead of
expanding the secret inside the run block (Sonar security hotspot).
- .gitignore the mysql/ data volume docker-compose mounts.
- Remove the container ExecAsync in the test factory whose arguments were wrong
(`-p mysql` prompts; `${...}` in C# is a literal) and whose result was ignored.
Docs
- README / package readme: PK fallback, accepted expression shapes, merge
rows-affected semantics, connection and transaction ownership, shadow props.
- csproj Description no longer claims compatibility with most databases.
- CLAUDE.md: key resolution, expression handling, ownership rule.
Tests: 40 -> 57. EdgeCaseTest (model-only) covers each generator change;
ConnectionOwnershipTest covers connection and ambient-transaction ownership
against MySQL. The two Log tests that asserted the old PK-only failure now
assert the fallback.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E63oHuiKenf9H6tcgvDXVG
…ge row-count claim EdgeCaseTest asserts on the generated SQL; these four send it to the server. The key-only merge (ON DUPLICATE KEY UPDATE t.Id = t.Id) is accepted and leaves matched rows alone; update and delete with no UniqueKeys option match on the primary key the library fell back to; a key named in IgnoreOnUpdate is used for the JOIN but not written. Running them showed the README's claim that a matched-but-unchanged row counts 0 was wrong: MySqlConnector connects with CLIENT_FOUND_ROWS (UseAffectedRows=false) by default, so such a row counts 1. Both readmes and the test now say so, and note that UseAffectedRows=true gives 0. The key-only entity lives in EdgeCaseDbContext, which has no table in the shared database, and EnsureCreated does nothing on a database that already has tables; it is created in a second database on the same container as root. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E63oHuiKenf9H6tcgvDXVG
|
hongjs
added a commit
that referenced
this pull request
Aug 23, 2026
Security: the one open vulnerability was publish_nuget.yml expanding
${{ secrets.NUGET_TOKEN }} inside the run block - the same finding fixed in
sonarcloud.yml by #34. The token now reaches the script through env.
Coverage: the library was at 99%; the 89.9% overall came from the API
project, where OrderRepository and Order had no test at all and
ProductRepository.SyncDataThenCommit's catch never ran. OrderRepositoryTest
sends the FK / DateOnly entity the benchmarks use through every repository
method against MySQL; EfCoreTest gains the commit-path failure.
Maintainability (the 10 new issues from #34): the ON / SET / ON DUPLICATE
KEY UPDATE loops in BulkCommand become two helpers built on string.Join,
column selection moves out of the update and merge generators so each drops
below the complexity limit, the TODO and commented-out FieldsToUpdate block
leave BulkOption (the design now lives in docs/roadmap.md, not yet
committed), EdgeCaseTest uses static readonly arrays, and two members
nothing read - ColumnInfo.IsKey, BatchData.SuccessCount - are removed along
with a redundant OpenAsync in ExecuteBatchDataAsync. Generated SQL is
unchanged: every exact-string assertion in BulkCommandTest still passes.
Tests: 61 -> 68.
Claude-Session: https://claude.ai/code/session_01E63oHuiKenf9H6tcgvDXVG
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Library
x => x.CreatedAtused to be compiled, run on a blank instance, and have DateTime's own properties read back, so IgnoreOnInsert/IgnoreOnUpdate/UniqueKeys silently did nothing. Onlyx => x.Propandx => new { x.A, x.B }are accepted; anything else throws.Idthat the SELECT no longer produced.SETon its own; merge of a key-only entity assigns a key to itself, MySQL's insert-if-absent idiom.Dependencies
CI / repo
-p mysqlprompts;${...}in C# is a literal) and whose result was ignored.Docs
Tests: 40 -> 57. EdgeCaseTest (model-only) covers each generator change; ConnectionOwnershipTest covers connection and ambient-transaction ownership against MySQL. The two Log tests that asserted the old PK-only failure now assert the fallback.
Claude-Session: https://claude.ai/code/session_01E63oHuiKenf9H6tcgvDXVG