microsoft / microsoft/mssql-rs
Unify SQL lexing between mssql-odbc and mssql-py-core
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 53
- Forks
- 14
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 137
Description
Problem statement
mssql-odbc's CodeScan (mssql-odbc/src/api/escape.rs) and mssql-py-core's rewrite_placeholders (mssql-py-core/src/async_parameters.rs) independently recognize SQL strings, quoted/bracketed identifiers, comments, and executable parameter markers. This duplication predates #542. That PR shares the scanner within ODBC, but not across the two bindings, leaving common lexical behavior susceptible to drift and duplicated fixes.
Track the cross-binding extraction separately from the escape-sequence and output-parameter work merged in #542.
Proposed solution
Extract a small shared span-based SQL lexer, potentially into mssql-tds, with explicit policies for behavior that intentionally differs between consumers. Select the final module/API placement during design rather than coupling the shared lexer to either binding.
Preserve the existing contracts:
- Python supports nested block comments,
%(name)splaceholders, and collision-free generated parameter names. - ODBC preserves msodbcsql's non-nesting block-comment behavior and canonical-extension quirks.
- Placeholder naming and binding, ODBC escape translation, and Python/ODBC value conversion remain in their respective layers; share lexical recognition, not those higher-level semantics.
Acceptance criteria:
- Both bindings use shared lexical primitives for strings, quoted/bracketed identifiers, comments, and identifying executable spans without duplicating the common scanning state machine.
- Existing observable behavior remains unchanged, including malformed-input handling, marker ordering, and text preservation.
- Reuse both existing test suites and add a shared conformance corpus, with explicit cases where Python and ODBC policies are expected to disagree.
- Retain coverage for markers inside literals/comments/identifiers, escaped delimiters, nested comments, ODBC canonical extensions, Python named placeholders, and generated-name collisions.
- Preserve the escape-scanner fuzz invariants introduced in #542, and avoid unnecessary allocations or copies in the shared scanning path.
- Validate both ODBC and the separately built
mssql-py-corecrate, including relevant E2E/parity tests, and document the policy boundaries.
Affected crate
Not applicable / Multiple: mssql-odbc, mssql-py-core, and potentially mssql-tds as the shared implementation location.
Alternatives considered
Keep separate scanners and duplicate fixes and tests: lowest immediate refactoring cost, but retains the risk of unintentional drift.
Force a single lexical policy across both bindings: not appropriate because nested-comment handling and ODBC canonical-extension behavior intentionally differ.
Expand #542 to perform the extraction: explicitly deferred to keep its correctness fixes separate from this behavior-preserving refactor.
Additional context
Follow-up to merged #542 and the review discussion proposing this extraction.
The discussion also mentioned reuse of multipart-identifier helpers in mssql-tds/src/sql_identifier.rs; that is a distinct, smaller reuse opportunity and should not broaden this lexer extraction unless required by its design.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by comparing CodeScan in mssql-odbc/src/api/escape.rs with rewrite_placeholders in mssql-py-core/src/async_parameters.rs, then read the #542 review discussion. Design the shared span-based lexer and its policy boundaries before choosing whether mssql-tds hosts it. Done means both bindings reuse shared lexical primitives, existing test and fuzz behavior remains unchanged, and the shared conformance corpus plus E2E/parity validation covers the stated differences.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust, sql
- Domain
- backend-api-design, databases
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100