microsoft / microsoft/mssql-rs

Unify SQL lexing between mssql-odbc and mssql-py-core

Open
#561 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
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)s placeholders, 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-core crate, 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.