Add DECIMAL Type Support to C# Language Extension#83
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds end-to-end SQL Server DECIMAL/NUMERIC support to the .NET Core C# language extension by introducing an ODBC-compatible SQL_NUMERIC_STRUCT representation in managed code and wiring conversions to/from SqlDecimal, plus expanding the native/managed test coverage for decimal parameters and columns.
Changes:
- Introduces
SqlNumericHelperwithSQL_NUMERIC_STRUCTlayout and conversion helpers to/fromSqlDecimal. - Updates parameter and dataset marshalling to support
SqlDecimal/SQL_C_NUMERICfor input columns, output columns, and OUTPUT parameters. - Adds new native and managed tests and updates test harness templates/instantiations for
SQL_NUMERIC_STRUCT.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
language-extensions/dotnet-core-CSharp/test/src/native/CSharpInitParamTests.cpp |
Adds InitParam template specialization for SQL_NUMERIC_STRUCT to pass precision/scale to InitParam. |
language-extensions/dotnet-core-CSharp/test/src/native/CSharpExtensionApiTests.cpp |
Adds InitializeColumns specialization for numeric columns to use precision rather than sizeof. |
language-extensions/dotnet-core-CSharp/test/src/native/CSharpExecuteTests.cpp |
Adds explicit template instantiation for executing numeric column tests. |
language-extensions/dotnet-core-CSharp/test/src/native/CSharpDecimalTests.cpp |
Adds a new native test suite covering decimal params, output params, and decimal columns. |
language-extensions/dotnet-core-CSharp/test/src/managed/Microsoft.SqlServer.CSharpExtensionTest.csproj |
Updates build output paths and adds Microsoft.Data.SqlClient dependency for managed tests. |
language-extensions/dotnet-core-CSharp/test/src/managed/CSharpTestExecutor.cs |
Adds managed executors to drive decimal OUTPUT parameter and precision-overflow scenarios. |
language-extensions/dotnet-core-CSharp/test/include/CSharpExtensionApiTests.h |
Adds max precision constant and a helper to build SQL_NUMERIC_STRUCT test values. |
language-extensions/dotnet-core-CSharp/src/managed/utils/SqlNumericHelper.cs |
New utility for SQL_NUMERIC_STRUCT layout plus conversion logic to/from SqlDecimal. |
language-extensions/dotnet-core-CSharp/src/managed/utils/Sql.cs |
Adds mapping and size metadata for NUMERIC/DECIMAL (SqlDecimal + SQL_NUMERIC_STRUCT). |
language-extensions/dotnet-core-CSharp/src/managed/Microsoft.SqlServer.CSharpExtension.csproj |
Adjusts output path defaults and adds Microsoft.Data.SqlClient package reference. |
language-extensions/dotnet-core-CSharp/src/managed/CSharpParamContainer.cs |
Adds NUMERIC param ingestion and output replacement via SqlDecimal + struct conversion. |
language-extensions/dotnet-core-CSharp/src/managed/CSharpOutputDataSet.cs |
Adds result column extraction path for NUMERIC/DECIMAL into SQL_NUMERIC_STRUCT[]. |
language-extensions/dotnet-core-CSharp/src/managed/CSharpInputDataSet.cs |
Adds input column ingestion path for NUMERIC/DECIMAL into PrimitiveDataFrameColumn<SqlDecimal>. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
SicongLiu2000
left a comment
There was a problem hiding this comment.
Review Summary
3 bugs found that should be fixed before merge, plus test coverage gaps and minor issues.
Bugs (must fix)
FromSqlDecimal(SqlDecimal.Null)crash —value.Precision/value.Scaleaccessed beforeIsNullguard (see inline comment on SqlNumericHelper.cs L225-226)- Precision overflow check false positives —
AdjustScaleinflatesPrecisionproperty causing valid values to be rejected (see inline comment on SqlNumericHelper.cs L266) - Precision clamped to 38 but scale not reduced — produces
DECIMAL(38,30)that rejects valid integer digits (see inline comment on CSharpOutputDataSet.cs L260-262)
Test coverage gaps
- No value round-trip test:
GetDecimalOutputParamTestchecks structural properties (precision range, sign ∈ {0,1}) but never checks thatval[16]bytes encode the expected number. A byte-ordering or indexing bug inToSqlDecimal/FromSqlDecimalwould pass all existing tests. - No all-NULL column test:
ExtractNumericColumnwould producemaxIntDigits=0, and if metadataDecimalDigits=0, precision becomes 0, clamped to 1 →DECIMAL(1,0)regardless of declared type. DecimalMixedPrecisionScaleInputTestonly checks merged metadata (precision, scale), not actual output data values after scale normalization — anAdjustScalebug (truncating vs zero-padding) would go undetected.CSharpTestExecutorDecimalPrecisionOverflowis defined but never called from any C++ test (see inline comment).
Other
- README (
language-extensions/dotnet-core-CSharp/README.mdline 10) still lists only the original data types —SQL_C_NUMERIC/DECIMAL/NUMERICshould be added to the supported types list.
Thanks Sicong for the thorough review of this PR. Regarding test gaps:
Will update the Readme.md too. |
SicongLiu2000
left a comment
There was a problem hiding this comment.
All 23 review threads addressed. The precision-clamping bug (scale not reduced when precision exceeds 38) is fixed in the latest push. LGTM.
75bcc5d to
f773323
Compare
…o PR #83 (C# extension decimal support) **Why is this change being made?** The C# Language Extension OSS repo ([microsoft/sql-server-language-extensions](https://github.com/microsoft/sql-server-language-extensions)) merged [PR #83: Add DECIMAL Type Support](#83), which adds full SQL Server DECIMAL/NUMERIC data type support to the .NET Core C# language extension. This change updates the internal `Data-SQL-Language-Extensions` submodule pointer to consume the merged commit. **What does the change do?** Updates the `sql-server-language-extensions` git submodule from commit `7e921b3` (pre-decimal) to [`7992ab3`](7992ab3) (merged PR 83 on main). The OSS PR introduced: - `SqlNumericHelper` utility for bidirectional conversion between ODBC `SQL_NUMERIC_STRUCT` and .NET `SqlDecimal` - Input/output column support for DECIMAL columns via `ExtractNumericColumn` in `CSharpOutputDataSet` - INPUT/OUTPUT parameter support for DECIMAL parameters in `CSharpParamContainer` - 13 decimal-specific unit tests and README update **How is the change tested?** - 71/71 unit tests pass in the OSS repo (Google Test) - E2E TestShell tests will be added in DsMainDev PR 2030890 (Covering full precision range, mixed types, NULL handling, 38-digit values)
This PR branch is based on sicongliu's branch which predates PR microsoft#83 (DECIMAL Type Support on main). PR microsoft#83 added Microsoft.Data.SqlClient 5.2.2 as a required runtime dependency for SqlDecimal handling. Without it, sp_execute_external_script fails with HRESULT 0x80004004 when any script touches DECIMAL types.
Summary
This PR implements full support for SQL Server
DECIMALtype in the C# language extension, enabling seamless conversion between SQL Server's 19-byteSQL_NUMERIC_STRUCTformat and .NET'sSqlDecimaltype. The implementation introduces a newSqlNumericHelperutility class and simplifies existing code by leveragingSqlDecimal's built-in capabilities.Why These Changes?
Problem: The C# language extension lacked proper support for SQL Server's
DECIMALtypes, which are critical for financial, scientific, and precision-sensitive applications. Without this support:DECIMALparameters couldn't be called from C# extensionsDECIMALcolumns couldn't be properly processedDECIMALreturned incorrect or corrupted valuesSolution: Implement bidirectional conversion between SQL Server's native
SQL_NUMERIC_STRUCT(ODBC 19-byte format) and .NET'sSqlDecimaltype, with proper handling of:SqlDecimal.NullWhat Changed?
1. SqlNumericHelper.cs
Created a comprehensive utility class for DECIMAL conversions with five core methods:
ToSqlDecimal(SqlNumericStruct): Converts SQL 19-byte struct →SqlDecimalFromSqlDecimal(SqlDecimal, precision, scale): ConvertsSqlDecimalto SQL structSqlDecimal.AdjustScale()when neededSqlDecimal.Precisionproperty (auto-updated by framework)ToSqlDecimalFromPointer(SqlNumericStruct*): Unsafe pointer version for OUTPUT parametersSqlDecimal.Null)2. CSharpDecimalTests.cpp
Added comprehensive test coverage with 8 new decimal-specific tests:
GetDecimalOutputParamTestDecimalPrecisionScaleTestDecimalBoundaryValuesTestDecimalStructLayoutTestGetDecimalInputColumnsTestGetDecimalResultColumnsTestDecimalColumnsWithNullsTestDecimalHighScaleTestTest Infrastructure Updates
3. CSharpTestExecutor.cs
Added managed test execution helper for decimal scenarios:
ExecuteDecimalInputOutput: Tests input columns + OUTPUT parametersExecuteDecimalResultSet: Tests decimal return columns4. CSharpExtensionApiTests.h/cpp
Extended C++ test framework with decimal test declarations and utility functions.
5. CSharpInitParamTests.cpp
Added
InitNumericParamTestfor parameter initialization validation.6. CSharpExecuteTests.cpp
Integrated decimal tests into main test suite execution.
10. Microsoft.SqlServer.CSharpExtension.csproj
Added
SqlNumericHelper.csto build configuration.11. Microsoft.SqlServer.CSharpExtensionTest.csproj
Added
CSharpTestExecutor.csto test project.Checklist