Skip to content

FIX: Use normalized size for DAE selection - #778

Closed
ttk (Theekshna) wants to merge 1 commit into
mainfrom
theekshna-fix-numeric-dae-selection
Closed

ttk (Theekshna) wants to merge 1 commit into
mainfrom
theekshna-fix-numeric-dae-selection

Conversation

@Theekshna

Copy link
Copy Markdown
Contributor

Reference: #736

Summary

Use normalized and clamped parameter metadata when selecting data-at-execution binding. This prevents oversized DECIMAL/NUMERIC declarations from incorrectly streaming short formatted Decimal values while preserving DAE for genuinely oversized character and binary declarations.

Validation

  • Built the x64 native extension with mssql_python\pybind\build.bat x64
  • python -m black --check --line-length=100 tests\test_023_execute_path_parity.py
  • python -m pytest -q tests\test_000_dependencies.py (35 passed, 3 skipped)
  • Focused live SQL tests require DB_CONNECTION_STRING; unavailable in this worktree

Copilot AI lite review requested due to automatic review settings September 10, 2026 18:22
@github-actions github-actions Bot added the pr-size: small Minimal code update label Sep 10, 2026

Copilot AI 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.

🟢 Approval recommended

The change is narrowly scoped, aligns with the stated fix intent, and includes targeted test coverage updates for the affected behavior.

Pull request overview

This PR adjusts the native parameter override path (setinputsizes) so data-at-execution (DAE) streaming decisions are based on the normalized/clamped parameter metadata (as stored in ParamInfo) rather than the raw caller-provided columnSize object. This specifically prevents oversized DECIMAL/NUMERIC declarations from accidentally forcing DAE when the value is formatted to a short string, while keeping DAE behavior for genuinely large character/binary declarations.

Changes:

  • Switch DAE gating in ApplyInputSizeOverride from comparing the raw columnSize PyObject to comparing info.columnSize (post normalization/clamp).
  • Add a regression test for setinputsizes-driven character DAE and clarify the intent of the oversized numeric metadata test.
File summaries
File Description
mssql_python/pybind/param_detect.hpp Uses normalized/clamped info.columnSize for DAE selection after applying input-size overrides, avoiding oversized numeric metadata triggering DAE.
tests/test_023_execute_path_parity.py Adds a setinputsizes character-size regression test and updates the numeric clamp test’s intent description to match the DAE-selection fix.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

Copy link
Copy Markdown

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

82%


📈 Total Lines Covered: 8191 out of 9889
📁 Project: mssql-python


Diff Coverage

Diff: main...HEAD, staged and unstaged changes

  • mssql_python/pybind/param_detect.hpp (100%)

Summary

  • Total: 2 lines
  • Missing: 0 lines
  • Coverage: 100%

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)
mssql_python.pybind.performance_counter.hpp: 0.7%
mssql_python.pybind.logger_bridge.cpp: 57.9%
mssql_python.pybind.ddbc_bindings.h: 61.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.row.py: 77.6%
mssql_python.pybind.ddbc_bindings.cpp: 77.7%
mssql_python.pybind.connection.connection_pool.cpp: 81.8%
mssql_python.logging.py: 85.5%
mssql_python.helpers.py: 89.3%
mssql_python.pooling.py: 90.1%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-size: small Minimal code update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants