Skip to content

fix(collation): support legacy CP437 and CP850 code pages - #452

Merged
MattJackson merged 3 commits into
tiberius-rs:mainfrom
t8y2:dev/support-cp437-cp850
Sep 25, 2026
Merged

MattJackson merged 3 commits into
tiberius-rs:mainfrom
t8y2:dev/support-cp437-cp850

Conversation

@t8y2

@t8y2 t8y2 commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Support CP437 SQL collations (sort IDs 30–35) and CP850 SQL collations (40–45, 49, and 55–61) with small internal single-byte codecs.
  • Decode CHAR, VARCHAR, TEXT, and intrinsic CHAR/VARCHAR values inside SQL_VARIANT; use the same codecs for CHAR/VARCHAR/TEXT bulk encoding.
  • Preserve the existing public Collation::encoding() API and built-in encoding_rs behavior. The public accessor still returns an error for DOS code pages because encoding_rs::Encoding cannot represent them; internal row/bulk paths now support them.
  • Keep unknown collations and unrepresentable outgoing characters as errors. No dependency changes.

Why

For example, SQL_1xCompat_CP850_CI_AS reports LCID 0x409 and sort ID 49. These SQL sort IDs already appear in the collation mapping, but their arms are commented out because encoding_rs does not implement CP437/CP850. Consequently, even valid row values fail with unsupported encoding before the caller can read them.

The sort-ID groups follow Microsoft's JDBC collation mappings. The byte tables follow the Unicode Consortium's CP437 and CP850 mappings. All 256 mappings per code page were cross-checked against those tables and Python's corresponding codec, including 0xFF → U+00A0.

Regression coverage

  • Raw TDS values fail against the base revision with unsupported CP437/CP850 encodings and pass with this patch.
  • Exhaustive decode/encode round trips of every byte for all supported DOS sort IDs, across CHAR, VARCHAR, and TEXT.
  • SQL_VARIANT character values, empty/NULL values, unrepresentable bulk values, encoded-byte length limits, built-in encodings, unknown sort IDs, and collation display names.

Validation

  • cargo fmt --all -- --check — passed.
  • cargo nextest run --lib --no-default-features — 869 passed.
  • cargo nextest run --lib --no-default-features --features tds73 — 913 passed.
  • cargo nextest run --lib --no-default-features --features rustls,chrono,time,tds73 — 998 passed.
  • cargo clippy --lib --no-default-features --features rustls,chrono,time,tds73 — completed with the same two existing warnings as the unmodified base (needless_borrow in client/connection.rs and large_enum_variant in client/tls.rs). -D warnings also fails on the base for those warnings.
  • Full doctests with --no-default-features --features rustls,chrono,time,tds73: 31 passed, 1 ignored, 7 failed because their examples require a live SQL Server (ConnectionRefused). An isolated run against unmodified base 7fd75bb reproduced exactly the same seven failures.

Unit/raw-TDS-frame and doctest validation only; no live SQL Server run.

@MattJackson

Copy link
Copy Markdown
Contributor

Thanks, this looks really solid. I ran it against real SQL Server 2022/2019/2017: all 24 CP437/CP850 collations in sys.fn_helpcollations() map to your sort-ID ranges, and every byte 0x00–0xFF across CHAR/VARCHAR/VARCHAR(MAX)/TEXT/SQL_VARIANT decoded to the same value as the server's own CONVERT(NVARCHAR). Bulk insert and parameter round-trips came back byte-exact too. A few things that could make it even better:

  1. It looks like the tests don't pin the table values. Changing one CP437 entry still passes, since the round-trip only checks decode against encode. Could we compare against a fixed expected string for each code page? A small live test in tests/query.rs could be worth it too. A temp table with COLLATE SQL_Latin1_General_CP850_CI_AS and N'…' values works without creating a database.
  2. legacy_codepages_reject_unrepresentable_bulk_values also seems to pass on main, because main's "unsupported encoding" error is the same type. Matching the "unrepresentable character" message would let it prove the new path.
  3. Optional: in a quick benchmark, a fast path for ASCII plus a small lookup table encoded about 2–5× faster on non-ASCII text. Not a blocker.
  4. A small follow-up idea: a Collation::code_page() accessor, since encoding() still can't represent these code pages.

Please let me know if I'm wrong on any assumptions. I look forward to working with you on this!

@t8y2

t8y2 commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for checking the collations and pointing out the gaps in the assertions. Updated in d68ce87.

The all-byte regression now compares against fixed expected Unicode strings for CP437 and CP850, rather than relying only on a round trip. Swapping two table entries now fails that test. The bulk-input regression also requires the exact unrepresentable character error, so an unsupported-code-page error cannot make it pass.

I also added query regression cases for both code pages across CHAR, VARCHAR, VARCHAR(MAX), TEXT, and SQL_VARIANT, including parameter round trips and checks of the stored bytes.

The library suite passes with 869 tests under --no-default-features and 998 with rustls,chrono,time,tds73. I kept the encoder optimization and public accessor ideas separate from this correctness-focused follow-up.

Client::bulk_insert* declared each column of the INSERT BULK column
list as `[name] type` without a collation. SQL Server reads the bulk
data of a char, varchar or text column declared that way in the
database default collation and converts it to the column's collation,
so text bulk-loaded into a column whose collation differs from the
database default was stored corrupted. In a CP1252 database,
"Привет" bulk-loaded into a Cyrillic_General_CI_AS column was stored
as "I?eaao", and every byte 0x80-0xFF of text bulk-loaded into a
SQL_Latin1_General_CP437_BIN or SQL_1xCompat_CP850_CI_AS column was
stored as a different byte: CP437 "é" (0x82) became "," (0x2C).

The INSERT BULK column list now declares char, varchar, text, nchar,
nvarchar and ntext columns with ` COLLATE <name>`, as SqlClient's
SqlBulkCopy does. The collation names come from
`EXEC <catalog>..sp_tablecollations_100 N'<schema>.<table>'`, run in
the batch of the column metadata query, so bulk_insert takes no extra
round trip. It runs in tempdb for a # temp table and in the catalog the
table name gives otherwise; before SQL Server 2008 the procedure is
sp_tablecollations_90. Columns are matched to their collation by name.
A collation name that is not ASCII letters, digits and `_` fails the
bulk insert with Error::Protocol instead of reaching the statement.

@MattJackson MattJackson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for d68ce87. Both fixes do exactly what I hoped. I swapped the first two CP437 table entries and legacy_codepages_match_unicode_mappings_for_every_byte failed right away, and with the new message check the bulk-reject test now fails on main as it should. fmt, clippy, MSRV 1.88, all 10 integration-matrix builds with -Dwarnings and the lib suites are all green, and your new query test passes on SQL Server 2022, 2019 and 2017.

I also owe you a correction. In my first comment I said bulk insert came back byte-exact, and that was wrong. The cause isn't your code, though. Main's INSERT BULK column list never declares a COLLATE, so SQL Server reads char/varchar/text bulk data in the database's default code page and converts it to the column's. That already corrupts bulk loads into any non-default-collation column on main (a Cyrillic column turns Привет into I?eaao), and CP437/CP850 columns hit the same bug: every byte 0x80–0xFF came back different (CP437 é was stored as ,).

Rather than have you chase that, I pushed a commit on top of your branch (dbb20e3). It declares each character column's collation in INSERT BULK, the way SqlClient's SqlBulkCopy does, and adds a live test that bulk-loads every high byte into CP437 and CP850 columns and checks the stored bytes. That test fails without the fix and passes with it on 2022, 2019 and 2017. With it, the full bulk (132) and query (200) suites pass on all three.

Approving. Deferring the fast path and the code_page() accessor is completely fine. Feel free to look over my commit, and tell me if you'd rather handle it differently.

@MattJackson
MattJackson merged commit 75d7afc into tiberius-rs:main Sep 25, 2026
37 checks passed
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.

2 participants