[mssql] Report transactional SQL to onSqlCommand - #307
Open
nltgpeterskoglund wants to merge 1 commit into
Open
nltgpeterskoglund wants to merge 1 commit into
nltgpeterskoglund wants to merge 1 commit into
Conversation
The Persister gives the driver-level Transaction an execute command of its own, which never went through getWrappedCommand. Since the JSON persister does its work inside that transaction, the callback stayed silent for both load() and save(), where the other database Persisters report every statement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This branch has not been deployed
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
1 participant
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.
Follow-up to #295, as offered there.
onSqlCommandnever fires for the mssql Persister.createMsSqlPersistergives the driver-levelTransactionan execute command of its own, and that one never passes throughgetWrappedCommand— so the statements the JSON persister runs inside the transaction bypass the callback entirely. For the samesave(),better-sqlite3reports six statements and mssql reports none, onload()as well assave().The unit tests did not catch it because they drive
createCustomMsSqlPersisterdirectly, without anexecuteTransaction, which falls back to the shared implementation that does use the wrapped command. This adds a test that goes through the real entry point with a stand-in forConnectionPool, so the transaction path is covered. I checked that it fails without the one-line change and passes with it.Also run against SQL Server 2022 in a container:
mssql.test.tsand the mssql variants ofjson.test.tsandmergeable-json.test.ts, 72 tests, all green.This has been there since the original branch, so it is a fix to my own code rather than a regression. Based on
main; the two files are identical onbeta, so it applies there just as well if that is the better home for it.🤖 Generated with Claude Code