FIX: False positive qmark detection for ? inside bracketed identifiers, string literals and comments#465
Open
FIX: False positive qmark detection for ? inside bracketed identifiers, string literals and comments#465
Conversation
Contributor
There was a problem hiding this comment.
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) inparameter_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.
jahnvi480
force-pushed
the
jahnvi/qmark_escape_char
branch
from
aa1fdeb to
3b3a687
Compare
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
📋 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
|
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
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
_skip_quoted_contexthelper inparameter_helper.pyto 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._has_unquoted_question_marksfunction to detect real?placeholders only outside quoted contexts, using the new context-skipping logic.detect_and_convert_parametersto use_has_unquoted_question_marksfor parameter style mismatch detection, preventing false positives when?appears inside bracketed identifiers or other quoted contexts.Testing improvements
_skip_quoted_contextand_has_unquoted_question_marks, covering all relevant SQL quoting and commenting scenarios, including edge cases like unclosed quotes/brackets.?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