FIX: record correct UTF-16 code-unit length for every string parameter during native detection - #759
Draft
Gaurav Sharma (bewithgaurav) wants to merge 10 commits into
Draft
FIX: record correct UTF-16 code-unit length for every string parameter during native detection#759Gaurav Sharma (bewithgaurav) wants to merge 10 commits into
Gaurav Sharma (bewithgaurav) wants to merge 10 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
reject invalid Decimal subclass formatting before replacing the parameter so native execution preserves the Python format contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reconcile #742 (bind Decimal as SQL_NUMERIC regardless of value, GH-740) with the native setinputsizes migration: - param_detect.hpp: drop the automatic MONEY/SMALLMONEY VARCHAR shortcut; every finite Decimal binds SQL_NUMERIC natively. FormatDecimalParam stays for the setinputsizes DECIMAL override only. - cursor.py: _create_parameter_types_list forwards decimal_as_numeric to _map_sql_type; the parameterless else-branch keeps DDBCSQLExecDirect and drops the deleted DDBCSQLExecuteLegacy block (GH-740 fix now happens in native detection). - test_023: keep both suites; narrow test_decimal_format_must_return_string to the setinputsizes override, the only path that still formats Decimals after GH-740. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
native detection left utf16Len at 0 for inline strings, time->isoformat strings, and setinputsizes-formatted strings; only the DAE branch set it. a later change will size a per-batch wide-char arena from utf16Len, where a 0 would undersize the slice and corrupt the heap. compute it (surrogate pairs counted as two units) after any in-place normalization at every site a string binds wide. no behaviour change: nothing reads utf16Len yet. expose it read-only and add a DetectParamTypesForTesting hook so the contract is unit-tested without a live server. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # mssql_python/cursor.py # mssql_python/pybind/ddbc_bindings.cpp # mssql_python/pybind/param_detect.hpp
Gaurav Sharma (bewithgaurav)
changed the title
Gaurav Sharma (bewithgaurav)
changed the base branch from
bewithgaurav/native-setinputsizes
to
main
September 8, 2026 18:52
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Work Item / Issue Reference
Summary
During native parameter detection,
ParamInfo.utf16Lenrecords how many UTF-16 code units a string parameter occupies, which is exactly what the wide-character binder writes for that value. Before this change, only the long-string data-at-execution path set the field; inline strings,timevalues normalized throughisoformat, andsetinputsizes-formatted strings all left it at0.That gap is a latent correctness bug in the parameter metadata: any consumer that trusts
utf16Lento size a wide-character buffer would read0for the common cases and undersize the allocation. The current production binder does not read the field yet, so there is no user-visible failure today; this change closes the gap defensively so the metadata is correct at every site a string binds wide, before anything starts depending on it.The length is computed after any in-place normalization (time and Decimal formatting), and supplementary Unicode characters above U+FFFF are counted as the two code units of their surrogate pair. The field is exposed read-only, and a
DetectParamTypesForTestinghook lets the detection contract be asserted without a live SQL Server.This is a metadata correctness fix only. It is not a performance change: arena allocation and cached parameter bindings are out of scope and land in separate pull requests.
No-Regression Check
utf16Lenis computed inside the hot detection path, so these local timings confirm the added work does not regress detection rather than claim any speedup. macOS arm64, Python 3.13, release builds (-O3 -DNDEBUG), 2,000 parameters per call, median of 270 timed samples after 30 warmups. Both builds include the same test hook, so the numbers cover list copying and result marshalling, not native detection alone.Both deltas sit inside run-to-run spread, so this is a wash. These are preliminary local observations, not a formal guarantee; profiler-based analysis is tracked separately.