FIX: False positive qmark detection for ? inside bracketed identifiers, string literals and comments by jahnvi480 · Pull Request #465 · microsoft/mssql-python · GitHub

Work Item / Issue Reference

AB#42937

GitHub Issue: #464

Summary

This pull request introduces a robust fix for detecting real parameter placeholders in SQL statements, specifically addressing false positives caused by question marks inside bracketed identifiers, string literals, quoted identifiers, and comments. The changes add context-aware scanning logic and comprehensive tests, ensuring that only actual parameter placeholders are flagged and handled. This resolves a bug where SQL containing ? inside brackets (e.g., [q?marks]) would incorrectly trigger parameter mismatch errors.

Core logic improvements

Added _skip_quoted_context helper in parameter_helper.py to accurately skip over single-line comments, multi-line comments, single-quoted string literals (with escaped quotes), double-quoted identifiers, and bracketed identifiers when scanning SQL for placeholders.
Added _has_unquoted_question_marks function to detect real ? placeholders only outside quoted contexts, using the new context-skipping logic.
Updated detect_and_convert_parameters to use _has_unquoted_question_marks for parameter style mismatch detection, preventing false positives when ? appears inside bracketed identifiers or other quoted contexts.

Testing improvements

Added extensive unit tests for _skip_quoted_context and _has_unquoted_question_marks, covering all relevant SQL quoting and commenting scenarios, including edge cases like unclosed quotes/brackets.
Added integration tests verifying that SQL with ? inside bracketed identifiers, string literals, and comments does not trigger parameter style mismatch errors, and that real placeholders are still detected correctly.

Test harness updates

Imported the new helper functions in the test module to facilitate direct testing.

Pull request overview

Fixes false positives in qmark (?) placeholder detection by making detect_and_convert_parameters() context-aware (skipping ? inside identifiers, string literals, and comments), addressing Issue #464 / AB#42937.

Changes:

Added SQL context skipping logic (_skip_quoted_context) and a context-aware qmark detector (_has_unquoted_question_marks) in parameter_helper.py.
Updated detect_and_convert_parameters() to use the new qmark detector for parameter style mismatch detection.
Added targeted unit + integration tests covering bracketed identifiers, strings, and comments containing ?.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
mssql_python/parameter_helper.py	Introduces context-aware scanning helpers and updates mismatch detection to avoid false positives from ? inside quoted contexts.
tests/test_015_pyformat_parameters.py	Adds unit/integration tests validating the new scanning behavior and the original reported bracketed-identifier scenario.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…, string literals, and comments The parameter style detection used a naive '?' in sql check which falsely flagged ? characters inside T-SQL bracketed identifiers ([q?marks]), string literals ('is this ok?'), double-quoted identifiers, and SQL comments as positional parameter placeholders. Changes: - Add _skip_quoted_context() helper to skip over SQL quoted contexts (brackets, single/double quotes, single-line and multi-line comments) - Add _has_unquoted_question_marks() that uses context-aware scanning to only detect ? characters that are actual qmark placeholders - Update detect_and_convert_parameters() to use the new context-aware check - Add 40 new unit tests covering the helper functions and integration scenarios Fixes: question mark in bracketed column name causes false positive 'Parameter style mismatch' error when using dict parameters.

📊 Code Coverage Report

🔥 Diff Coverage 100%	🎯 Overall Coverage 77%	📈 Total Lines Covered: 5645 out of 7324 📁 Project: mssql-python

Diff Coverage

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

mssql_python/parameter_helper.py (100%)

Summary

Total: 67 lines
Missing: 0 lines
Coverage: 100%

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)

mssql_python.pybind.logger_bridge.hpp: 58.8%
mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.row.py: 66.2%
mssql_python.pybind.ddbc_bindings.h: 67.8%
mssql_python.pybind.ddbc_bindings.cpp: 69.7%
mssql_python.pybind.connection.connection.cpp: 75.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.cursor.py: 84.8%
mssql_python.__init__.py: 84.9%

🔗 Quick Links

⚙️ Build Summary	📋 Coverage Details
View Azure DevOps Build	Browse Full Coverage Report

need to fix an edge case

Copilot AI review requested due to automatic review settings March 3, 2026 10:05

github-actions Bot added the pr-size: medium Moderate update size label Mar 3, 2026

Copilot started reviewing on behalf of Jahnvi Thakkar (jahnvi480) March 3, 2026 10:06 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

Comment thread mssql_python/parameter_helper.py Outdated

Comment thread mssql_python/parameter_helper.py Outdated

Comment thread mssql_python/parameter_helper.py Outdated

Comment thread tests/test_015_pyformat_parameters.py

Comment thread tests/test_015_pyformat_parameters.py

Jahnvi Thakkar (jahnvi480) force-pushed the jahnvi/qmark_escape_char branch from aa1fdeb to 3b3a687 Compare March 3, 2026 10:11

Gaurav Sharma (bewithgaurav) requested changes Mar 9, 2026

View reviewed changes

Comment thread mssql_python/parameter_helper.py

Adding nested comment edge case

dfad73e

Jahnvi Thakkar (jahnvi480) requested a review from Gaurav Sharma (bewithgaurav) March 9, 2026 09:31