microsoft / microsoft/mssql-rs
Bulk copy: destination table name is not quoted in the INSERT BULK command (column names are)
- Dominant language
- Rust
- Stars
- 53
- Forks
- 14
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 137
Description
### Describe the bug
While doing some ad-hoc testing of bulk copy with awkward-but-legal table names, I ran into an inconsistency in how identifiers are quoted inside the bulk copy path.
`fetch_table_metadata` is careful about this — it parses the destination with `parse_multipart_identifier` and rebuilds it with `build_multipart_name`, so each part comes out bracket-quoted:
```rust
// mssql-tds/src/connection/metadata_retriever.rs
let parts = parse_multipart_identifier(table_name, false)?;
...
let full_name = build_multipart_name(&parts);
let escaped_full_name = escape_string_literal(&full_name);
```
But `build_insert_bulk_command` interpolates the same string verbatim:
```rust
// mssql-tds/src/message/bulk_load.rs:1044
let mut command = format!("INSERT BULK {table_name} (");
for (i, col_meta) in column_metadata.iter().enumerate() {
...
// :1052 — the very next line
command.push_str(&format!("[{}] ", col_meta.column_name));
```
So within a single generated statement, column names are bracket-quoted and the table name is not.
The practical effect is that the two halves of one operation disagree: metadata retrieval succeeds for a table whose name needs delimiting, and then the `INSERT BULK` that immediately follows is malformed.
Call path, for reference:
- `BulkCopy::new(client, table_name)` stores the caller's string as-is on `self.table_name`
- `BulkCopy::retrieve_metadata` → `fetch_table_metadata(&self.table_name, ..)` — parses and escapes ✔
- `BulkCopy::write_to_server_zerocopy` → `execute_bulk_load_streaming_zerocopy(self.table_name.clone(), ..)` → `build_insert_bulk_command(&table_name, ..)` — verbatim ✘
Nothing in the public API suggests callers should pre-bracket the name. The doc comment on `BulkCopy::new` says:
```
/// * `table_name` - Name of the destination table (can include schema: "dbo.Users")
```
and the example passes a bare `"MyTable"`. Given that column names passed through the same function are bracket-quoted automatically, the natural assumption is that the table name is handled the same way.
Note this only affects the `INSERT BULK` statement — I didn't find an `ORDER(...)` hint in this code path, so the option list (`KEEP_NULLS`, `TABLOCK`, `CHECK_CONSTRAINTS`, `FIRE_TRIGGERS`) is all fixed strings and isn't affected.
### Steps to reproduce
`build_insert_bulk_command` is a pure function, so the cheapest repro is an in-crate unit test — no server needed:
```rust
// mssql-tds/src/message/bulk_load.rs
let cmd = build_insert_bulk_command(
"bulk load staging",
&[/* one int column named "id" */],
&BulkCopyOptions::default(),
).unwrap();
// produces: INSERT BULK bulk load staging ([id] int)
// expected: INSERT BULK [bulk load staging] ([id] int)
```
End to end against a live server:
1. Create a table whose name needs delimiting:
```sql
CREATE TABLE [bulk load staging] ([id] INT NOT NULL, [name] NVARCHAR(50) NULL);
```
2. Bulk copy into it using the plain, undelimited name:
```rust
let mut bulk_copy = BulkCopy::new(&mut client, "bulk load staging");
bulk_copy.write_to_server_zerocopy(rows).await?;
```
3. Metadata retrieval succeeds; the subsequent `INSERT BULK` fails.
The same shape applies to a couple of other ordinary cases:
```sql
CREATE TABLE [Order] ([id] INT NOT NULL); -- reserved word
CREATE TABLE [weird]]name] ([id] INT NOT NULL); -- name containing ]
```
```rust
BulkCopy::new(&mut client, "Order");
BulkCopy::new(&mut client, "weird]name");
```
### Expected behavior
The table name is quoted per part in the `INSERT BULK` command, consistent with how the same crate already handles it during metadata retrieval, and consistent with the column names in the same statement. Any table name that is legal in SQL Server should work when passed undelimited, without the caller needing to know to add brackets.
This is also what .NET `SqlBulkCopy` does — it parses `DestinationTableName` and brackets each part before building the command:
```csharp
// SqlBulkCopy.cs
string[] parts = MultipartIdentifier.ParseMultipartIdentifier(DestinationTableName, ...);
updateBulkCommandText.AppendFormat("insert bulk {0} (", ADP.BuildMultiPartName(parts));
```
### Actual behavior
The raw string is spliced into the command text, so a name that needs delimiting produces a malformed statement and the server rejects it, e.g.:
```
INSERT BULK bulk load staging ([id] int, [name] nvarchar(50))
```
which SQL Server parses as `INSERT BULK bulk` followed by unexpected tokens.
Note: I traced this by reading the code at `91b43e7` rather than running it against a live server, so I can't quote the exact error text or message number — the malformed command text above is what `build_insert_bulk_command` returns, which is the part I verified.
### Version
mssql-tds 0.1.0 (commit 91b43e7)
### Affected crate
mssql-tds
### Environment
N/A — found by code reading, not tied to a particular OS/server build.
### Additional context
The crate already has everything needed for the fix in `mssql-tds/src/sql_identifier.rs`; it just isn't applied at this one site. Something like:
```rust
use crate::sql_identifier::{build_multipart_name, parse_multipart_identifier};
let parts = parse_multipart_identifier(table_name, false)?;
let quoted_table = build_multipart_name(&parts);
let mut command = format!("INSERT BULK {quoted_table} (");
```
Worth noting for backwards compatibility: this shouldn't break callers who already pass a delimited name. `parse_multipart_identifier` consumes the brackets and returns the unquoted parts, so `[dbo].[My Table]` round-trips back to `[dbo].[My Table]` rather than being double-quoted, and `dbo.Users` becomes `[dbo].[Users]`.
For cross-reference, `github.com/microsoft/go-mssqldb` had the same asymmetry in its bulk copy path (quoted column names, unquoted table name) — https://github.com/microsoft/go-mssqldb/pull/416 is the in-flight fix there, if it's useful as a comparison for the compatibility edge cases.
Contributor guide
Research direction
Start in mssql-tds/src/message/bulk_load.rs at build_insert_bulk_command and compare its table-name handling with fetch_table_metadata in mssql-tds/src/connection/metadata_retriever.rs. Read mssql-tds/src/sql_identifier.rs, then run or add the in-crate unit test for a space-containing table name; done means generated INSERT BULK text quotes each table-name part while preserving quoted and multipart inputs.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100