feat(sqlserver): implement insert_record with IDENTITY_INSERT handling (#147)#215
Draft
saurabh500 wants to merge 1 commit into
Draft
feat(sqlserver): implement insert_record with IDENTITY_INSERT handling (#147)#215saurabh500 wants to merge 1 commit into
saurabh500 wants to merge 1 commit into
Conversation
TabularisDB#147) - Add build_insert_sql helper: plain INSERT or TRY/CATCH-wrapped IDENTITY_INSERT block that always turns the setting back OFF (including in the CATCH branch, since rollback does not revert it). - Add value_to_sql_param: dispatch on serde_json::Value variant to the bridge's typed ToSql impls, avoiding the bridge's blanket Value impl that NVARCHAR-stringifies every variant. - Add detect_identity_column: query sys.columns for is_identity=1. - Wire insert_record to detect identity, sort columns, derive needs_identity_insert from data keys, build SQL, bind via value_to_sql_param, and return ExecuteResult::total(). - Update the read-only test: insert_record now errors out on empty data rather than 'read-only'; delete and create_view unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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.
Summary
Implements
Driver::insert_recordfor the SQL Server driver with proper IDENTITY column handling. Closes #147.Changes
sqlserver/helpers.rs— new pure helperbuild_insert_sql(qualified, columns, wrap_identity_insert)that emits either a plainINSERT … VALUES (@P1, @P2, …)or, when an IDENTITY column is being supplied, wraps the insert inBEGIN TRY … BEGIN TRAN; SET IDENTITY_INSERT … ON; INSERT …; SET IDENTITY_INSERT … OFF; COMMIT TRAN; END TRY BEGIN CATCH IF @@TRANCOUNT > 0 ROLLBACK TRAN; SET IDENTITY_INSERT … OFF; THROW; END CATCH.IDENTITY_INSERTis session-scoped and not reverted by rollback, so the CATCH block explicitly turns it back OFF before re-raising.sqlserver/helpers.rs— new helpervalue_to_sql_param(&Value) -> Box<dyn ToSql>that dispatches on the JSON variant to the bridge's typed primitiveToSqlimpls (bool,i64,f64,String,Option::<String>::None). The bridge's blanketimpl ToSql for serde_json::ValueJSON-stringifies every variant intoNVARCHAR(4000)(filed upstream), which silently breaks inserts intobit/int/float/etc columns.sqlserver/introspection.rs— newdetect_identity_column(conn, schema, table)async fn +Q_GET_IDENTITY_COLUMNconstant. Queriessys.columnsfiltered byis_identity = 1for the resolvedobject_id.sqlserver/mod.rs— replaces the read-only stub with a real implementation:datadetect_identity_columnto learn the table's IDENTITY column (if any)needs_identity_insertbuild_insert_sqlwith the right wrappingvalue_to_sql_param, execute, returnexecute_result.total()Tests
helpers.rs:build_insert_sql(plain INSERT, identifier escaping, identity wrapping shape, custom target name)value_to_sql_paramdispatch acrossBool/Number(i64)/Number(f64)/String/Null/Array/Objectintrospection.rscoveringQ_GET_IDENTITY_COLUMNquery shapewrite_operations_error_out_in_phase1—insert_recordno longer errors out with "read-only" since the stub is gone; now asserts the empty-data validation message. Delete + create_view still assert "read-only".All 102 driver tests pass (
cargo test --lib drivers::sqlserver).Bridge limitation note
preview.3'sexecute()returns 0 for DML row counts due to an mssql-tds DONE token limitation (bridge issue #1). Callers should verify via SELECT, not the return value. This PR returns.total()as-is — accurate row count handling is tracked by the bridge upgrade work (PR for #145).Out of scope (followups)
readonly: trueonce update/delete are also implemented.Closes #147