Report 22003 for an unrepresentable numeric column value - #284
Conversation
numeric_source returned Option, so a decimal magnitude too wide for i128 collapsed into the same None as a column with no numeric interpretation, and the caller reported 07006 (restricted data type attribute violation) instead of 22003 (numeric value out of range). Fixes #235 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Corrects numeric conversion errors so unrepresentable decimal values report SQLSTATE 22003 instead of 07006.
Changes:
- Preserves
OutOfRangeversusRestrictedconversion failures. - Adds integer and floating-point regression tests for oversized decimals.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
🔗 Quick Links |
Saurabh Singh (saurabh500)
left a comment
There was a problem hiding this comment.
Traced the SQLSTATE decision back to the msodbcsql source before commenting. The core reclassification is right, and the blast radius is genuinely nil: the set of Ok values is byte-for-byte unchanged, both ConvError variants were already mapped by finish_typed_conv, and no caller branches on Restricted to select a fallback path. Tightly scoped fix.
Parity determination
From SqlClientDrivers/msodbcsql, master. Sql/Ntdbms/sqlncli/odbc/sqlcprot.h defines the mapping:
#define CVT_PREC IDS_22_003 // if loss of precision occurred.
#define CVT_ILLEGAL IDS_07_006 // if conversion is not supported.| Case | msodbcsql (sqlccnvt.cpp) |
this PR | Verdict |
|---|---|---|---|
| decimal out of range into an integer target | ConvertToFixed range checks return CVT_PREC |
22003 |
matches |
| non-numeric source (binary, guid) | default: arm returns CVT_ILLEGAL |
07006 |
matches |
wide magnitude into SQL_C_DOUBLE |
ConvertToFloat accumulates all four 32-bit words into a double and succeeds; its only CVT_PREC exit on that path is an undersized input buffer |
22003 error |
diverges |
So the integer half is exact parity, which is worth stating in the description per .github/instructions/mssql-odbc.instructions.md. The float half is the one I would like your read on, in the comment on the new test.
Three inline comments, all "should fix" rather than blocking. One nit not worth its own thread: the Decimal | Numeric arm is only ever exercised through Decimal, so a future split of that arm would go uncaught.
Verified locally at 9025f0c: the full mssql-odbc lib suite passes (566), and clippy --all-features --all-targets -D warnings is clean.
…asis A 4-word magnitude above i128::MAX has no exact scaled form, but msodbcsql's ConvertToFloat accumulates all four words into a double, so a float target now matches instead of reporting 22003. Four words is also all SQL_NUMERIC_STRUCT holds, so a wider magnitude has no counterpart to diverge from and stays 22003. Also documents the convert_float_c outcomes and pins the max-precision boundary. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Saurabh Singh (saurabh500)
left a comment
There was a problem hiding this comment.
Verified the parity fix: msodbcsql's ConvertToFloat Horner accumulation (Temp = Temp * 2^32 + word) lands on exactly 2^128, which is the same value u128::MAX as f64 rounds to — so the new test pins real parity rather than a coincidence, and ConvOk::Exact matches CVT_NO_ERROR on that path.
All four comments addressed, 567 tests pass, clippy and fmt clean at b9db4e5.
Description
numeric_sourceinmssql-odbc/src/api/fetch_convert.rsreturnedOption<NumericSource>, so every failure collapsed into a singleNone. The caller turned that intoConvError::Restricted, whichfinish_typed_convposts as 07006 ("restricted data type attribute violation").That is right for a column with no numeric interpretation (binary, guid, a date/time column into an integer target). It is wrong for a value that is numeric but cannot be represented, which is 22003 ("numeric value out of range").
numeric_sourcenow returnsResult<NumericSource, ConvError>, andnumeric_source_or_parsepropagates instead of re-wrapping, so its character-column arms (22018 / 22003) are unchanged. BothConvErrorvariants already existed and were already mapped to the right SQLSTATEs byfinish_typed_conv, so there is no new diagnostic plumbing. Both callers (convert_integer_c,convert_float_c) get the corrected SQLSTATE.Wide decimals
Review raised that the decimal arm was conflating two different failures, so they are now split:
i128::MAX, still within 4 words. Reachable: the decoder validates limb count but never the magnitude againstprecision, so 16 bytes of0xFFarrives intact from a non-conforming server. There is no exact scaled form, so it degrades to thef64reading. A float target then succeeds — matching msodbcsql'sConvertToFloat, which accumulates all four words into a double — while an integer target still reports 22003 because the value falls outside the integer range.SQL_NUMERIC_STRUCTis four words as well, so there is no msodbcsql counterpart here to diverge from.Parity statement: matches msodbcsql.
sqlcprot.hmapsCVT_PREC→IDS_22_003andCVT_ILLEGAL→IDS_07_006;sqlccnvt.cppConvertToFixedreturnsCVT_PRECfrom its range checks and reservesCVT_ILLEGALfor itsdefault:arm — the same split drawn here. Cited in code. No new row in the known-divergences table, since the one reachable divergence is now gone rather than documented.Reaching parity needed no
mssql-tdschange:DecimalParts::to_f64andmagnitude_bigare private, butmagnitude()ispuband returnsu128, which already covers the reachable case.Tests
decimal_limbs_are_reassembled_and_bounded— assertsOutOfRangefor the 5-word payload and for a 4-word magnitude abovei128::MAX, plus a new boundary case that the largest legaldecimal(38, 38)magnitude (10^38 - 1, ≈9.99e37 vsi128::MAX≈1.70e38) still keeps its exact form.wide_decimal_into_float_target_matches_msodbcsql— the reachable 4-word case converts on a float target.oversized_decimal_into_float_target_is_out_of_range— the defensive >4-word case still reports 22003.Related Issues
Fixes #235
Checklist
cargo bfmtpassescargo bclippypassescargo btestpasses (567mssql-odbclib tests pass locally viacargo test -p mssql-odbc --all-features --lib;cargo-nextestis not installed in this environment, so the full workspace nextest run is left to CI)numeric_sourceis private to the module)