Skip to content

feat(sqlserver): implement insert_record with IDENTITY_INSERT handling (#147)#215

Draft
saurabh500 wants to merge 1 commit into
TabularisDB:feat/sql-serverfrom
saurabh500:feat/147-identity-insert
Draft

feat(sqlserver): implement insert_record with IDENTITY_INSERT handling (#147)#215
saurabh500 wants to merge 1 commit into
TabularisDB:feat/sql-serverfrom
saurabh500:feat/147-identity-insert

Conversation

@saurabh500
Copy link
Copy Markdown
Contributor

Summary

Implements Driver::insert_record for the SQL Server driver with proper IDENTITY column handling. Closes #147.

Changes

  • sqlserver/helpers.rs — new pure helper build_insert_sql(qualified, columns, wrap_identity_insert) that emits either a plain INSERT … VALUES (@P1, @P2, …) or, when an IDENTITY column is being supplied, wraps the insert in BEGIN 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_INSERT is session-scoped and not reverted by rollback, so the CATCH block explicitly turns it back OFF before re-raising.
  • sqlserver/helpers.rs — new helper value_to_sql_param(&Value) -> Box<dyn ToSql> that dispatches on the JSON variant to the bridge's typed primitive ToSql impls (bool, i64, f64, String, Option::<String>::None). The bridge's blanket impl ToSql for serde_json::Value JSON-stringifies every variant into NVARCHAR(4000) (filed upstream), which silently breaks inserts into bit/int/float/etc columns.
  • sqlserver/introspection.rs — new detect_identity_column(conn, schema, table) async fn + Q_GET_IDENTITY_COLUMN constant. Queries sys.columns filtered by is_identity = 1 for the resolved object_id.
  • sqlserver/mod.rs — replaces the read-only stub with a real implementation:
    1. Validate non-empty data
    2. detect_identity_column to learn the table's IDENTITY column (if any)
    3. Sort columns deterministically and check whether the user-provided data includes the IDENTITY column → derive needs_identity_insert
    4. build_insert_sql with the right wrapping
    5. Bind params via value_to_sql_param, execute, return execute_result.total()

Tests

  • 9 new unit tests in helpers.rs:
    • 4 covering build_insert_sql (plain INSERT, identifier escaping, identity wrapping shape, custom target name)
    • 1 covering value_to_sql_param dispatch across Bool/Number(i64)/Number(f64)/String/Null/Array/Object
  • 1 new unit test in introspection.rs covering Q_GET_IDENTITY_COLUMN query shape
  • Updated write_operations_error_out_in_phase1insert_record no 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's execute() 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)

  • Testcontainers-gated integration tests covering plain INSERT, identity-provided INSERT, and CATCH-path rollback.
  • Removing manifest readonly: true once update/delete are also implemented.

Closes #147

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant