FIX: Correct GetInfoConstants IDs and ODBC result decoding

[Jahnvi Thakkar (jahnvi480)]

Work Item / Issue Reference

AB#48074

---

Summary

Correct all 11 misassigned SQLGetInfo information-type IDs and add the standard SQL_TIMEDATE_FUNCTIONS spelling at 52, preserving SQL_DATETIME_FUNCTIONS as the canonical enum name.

• Retain all 11 misplaced enum attributes and their original values as deprecated compatibility members. Preserve module-level imports and get_info_constants() lookups, expose the appropriate ConstantsDDBC groups, and add the standard SQL conformance result flags.
• Use one immutable, module-level ODBC return-type registry with exact numeric widths, including pointer-sized handles. Decode unsigned values without guessing; reject wrong-width or truncated buffers. Preserve non-byte floats/bools unchanged and safely convert decimal strings.
• Correct the five known unlisted standard IDs: SQL_DBMS_NAME (17), SQL_DBMS_VER (18), SQL_XOPEN_CLI_YEAR (10000), SQL_ASYNC_MODE (10021), and SQL_CREATE_ASSERTION (127).
• Add independent reference coverage for 102 non-deprecated enum names/aliases, 11 compatibility names, and five additional raw IDs. Cover exact forwarding, widths, unsigned boundaries, Unicode, legacy payloads, imports/stubs, enum pickles, connection-local results, and live native-payload comparisons. Replace blanket None skips with strict assertions and explicit Driver Manager-only exclusions.

Deprecation-first compatibility policy

The misplaced enum members remain available throughout 1.x, with removal deferred to 2.0 or later, only after maintainer approval and an explicit migration notice. No removal date is scheduled. This revision implements documented deprecation without custom enum-access hooks or runtime warnings.

Existing calls such as GetInfoConstants.SQL_ROW_NUMBER.value continue to work. The deprecated SQL_SQL92_*_SQL names retain 127/128/129; new code should use SQL_SC_SQL92_ENTRY=1, SQL_SC_FIPS127_2_TRANSITIONAL=2, SQL_SC_SQL92_INTERMEDIATE=4, and SQL_SC_SQL92_FULL=8 to interpret conformance results.

Deprecated names remain in get_info_constants() for dictionary-lookup compatibility, but are not valid information-type names. Their numeric values still request unrelated information; rejecting those integers would also reject legitimate colliding information types. The explicit decoding registry is independent of these compatibility names.

Correcting the actual information-type IDs intentionally changes incorrect results. Persisted raw IDs and value-based enum pickles for corrected members remain ambiguous and must be rebuilt from the intended names. Name-based enum pickles resolve retained names to their corrected values. Both formats are covered explicitly because the default reducer differs across Python versions. The existing SQL_DATETIME_FUNCTIONS.name is preserved.

Validation and limitations

• Windows x64, Python 3.13, Microsoft ODBC 18.6.2.1, SQL Server LocalDB: 1,235 focused tests passed, with three explicit Driver Manager-only skips. All 137 pickle cases also passed with the name-based default emulated locally. Previous full-suite run: 3,629 passed, 137 skipped, 42 deselected, and the same two previously reproduced baseline environment failures (password-mask expectation under integrated authentication; Windows long-path limit).
• Connection.getinfo focused coverage: 50/52 statements (96.2%) and 26/26 branch exits (100%), using repository exclusions. The two uncovered statements are the retained defensive exception handler in the unregistered binary fallback; native C++ coverage was not measured.
• Earlier workload checks exercised independent catalog changes, Unicode rows, unchanged transaction state, and 3,200 metadata calls across eight separate connections. The registry revision also passed live raw-ID, catalog-isolation, transaction-state, and Unicode-row scenarios.
• Cross-platform execution and the optional Rust ODBC provider have not been validated locally.

Scope

No native-code or dependency changes from this fix, no additional native calls, and no cross-connection metadata cache. Other unregistered raw IDs retain their legacy handling; this is not a universal SQLGetInfo overhaul. Correct IDs do not add support for Driver Manager-only handle queries. Standard Y/N character responses are retained, but nonstandard lowercase/binary responses no longer receive the old normalization.

The native-error contract is deliberately unchanged: unsupported-type, timeout, and connection-loss errors are logged and return None. Changing that contract is deferred to separate work. Providers can return cached metadata after connection loss, so getinfo() is not a connection-health check.

[Copilot]

🔵 Needs a closer look

It introduces intentional breaking API surface changes (removed enum members / corrected persisted values) plus core getinfo() decoding behavior changes that require an explicit human-reviewed compatibility/migration decision before merge.

Pull request overview

This PR corrects ODBC SQLGetInfo information-type IDs exposed via GetInfoConstants, updates Connection.getinfo() decoding to follow the ODBC-declared return types (string vs unsigned numeric), and adds a comprehensive regression test suite to prevent future transcription/decoding regressions (GH-769).

Changes:

• Fixed 11 misassigned GetInfoConstants IDs, added SQL_TIMEDATE_FUNCTIONS as the canonical name, and kept SQL_DATETIME_FUNCTIONS as an alias.
• Updated Connection.getinfo() to route known info-types via immutable lookup sets and decode numeric payloads as unsigned integers (with malformed-length rejection).
• Added extensive reference/decoder/export/stub and native-payload comparison coverage; updated changelog and strengthened existing assertions.

File summaries

File	Description
mssql_python/constants.py	Corrects GetInfoConstants IDs/aliases and moves non-information constants into ConstantsDDBC while keeping module-level exports.
mssql_python/connection.py	Adds explicit info-type routing sets and switches advertised numeric decoding to unsigned integers with stricter length validation.
mssql_python/mssql_python.pyi	Updates public stubs to reflect new/relocated exported constants and added aliases.
mssql_python/__init__.py	Keeps top-level exports aligned with the updated constants surface (including moved legacy constants and new alias).
tests/test_003_connection.py	Strengthens getinfo() assertions (notably requiring numeric SQL conformance to be an int).
tests/test_027_getinfo.py	Introduces a full ODBC reference table and end-to-end regression tests for enum IDs/aliases, decoding boundaries, exports/stubs, and native payload matching.
CHANGELOG.md	Documents the corrected IDs, decoding behavior changes, and the reclassification/migration guidance for non-information constants.

Review details

• Files reviewed: 7/7 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.

[Copilot]

🟡 Changes recommended

A modified test now fails on valid getinfo() contract behavior (None for unsupported info types), which could cause avoidable CI brittleness across providers/drivers.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Connection.getinfo() still silently coerces floats/bools via int(data) for numeric info types, which can truncate/alter values and contradict the intended “unsigned integer” contract for advertised SQLGetInfo numeric results.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

mssql_python/connection.py:1964

• For numeric GetInfoConstants, converting non-integer numerics with int(data) can silently change values (e.g., 1.9 → 1) and will also coerce booleans (since bool is a subclass of int). Since advertised SQLGetInfo numeric results are unsigned integers, it’s safer to only coerce true int (and digit-only strings) and otherwise return the value unchanged (or raise).

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Gaurav Sharma (bewithgaurav)]

the metadata fixes address the reported issue, but existing enum callers still break without an agreed migration approach. requesting changes until that compatibility decision is settled.

[Copilot]

🔵 Needs a closer look

It makes API- and behavior-affecting changes to public constants and getinfo() decoding semantics, and the PR explicitly notes a pending compatibility/migration policy decision before merge.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There are correctness issues to address (notably the broken logger.debug("warning", ...) call pattern) and a PR-description compatibility mismatch that should be reconciled before merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Connection.getinfo() currently accepts bool values as valid info_type (because bool is a subclass of int), which can silently map True/False to IDs 1/0 and should be explicitly rejected for correctness and consistency.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

mssql_python/connection.py:1973

• The integer type check for info_type just above this block uses isinstance(info_type, int), which will accept True/False and silently treat them as IDs 1/0. This is a common int/bool footgun and is inconsistent with _validate_timeout() earlier in this file, which explicitly rejects bool.

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Connection.getinfo() currently accepts boolean info_type values due to isinstance(..., int), which conflicts with the new stricter contract/tests and can silently map True to info type 1.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 1
• Review effort level: Lite

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 100%

🎯 Overall Coverage 83%

📈 Total Lines Covered: 8220 out of 9874 📁 Project: mssql-python

---

Diff Coverage

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

• mssql_python/connection.py (100%)
• mssql_python/constants.py (100%)

Summary

• Total: 58 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

[Gaurav Sharma (bewithgaurav)]

lgtm

[Copilot]

🔵 Needs a closer look

Connection.getinfo() should reject boolean values before integer validation.

Review details

Suppressed comments (1)

mssql_python/connection.py:1973

• bool is a subclass of int, so this validation still accepts True and False and forwards them as information types 1 and 0 instead of rejecting them as non-integer API inputs. That is inconsistent with the connection's existing integer validation, which explicitly excludes bool in _validate_timeout (connection.py:1090-1093), and can silently return unrelated metadata; exclude bool before the isinstance(info_type, int) check.

            logger.debug("Invalid info_type: %d. Must be non-negative.", info_type)

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Lite