Skip to content

Fix MSSQL passwords with special characters - #2658

Open
surajs-db wants to merge 4 commits into
mainfrom
fix/mssql-password-odbc
Open

surajs-db wants to merge 4 commits into
mainfrom
fix/mssql-password-odbc

Conversation

@surajs-db

Copy link
Copy Markdown

Changes

What does this PR do?

Fixes the Lakebridge MSSQL connection test for passwords containing ODBC-special characters, including ?.

When configuring an on-premises MSSQL source, a password containing ? was inserted directly into the ODBC connection string. The mssql_python parser interpreted the character as connection-string syntax and failed with:

ConnectionStringParseError: Incomplete specification: keyword '?' has no value (missing '=')

This prevented configure-database-profiler from completing the MSSQL connection test.

Relevant implementation details

Formats MSSQL password values with odbc_value before constructing the connection string, so special characters are treated as password data rather than connection-string delimiters.

Caveats/things to watch out for when reviewing:

Linked issues

Resolves #..

Functionality

  • added relevant user documentation
  • added new CLI command
  • modified existing command: databricks labs lakebridge ...
  • ... +add your own

Tests

  • manually tested
  • added unit tests
  • added integration tests

Signed-off-by: Suraj Shrestha <suraj.shrestha@databricks.com>
@surajs-db
surajs-db requested a review from a team as a code owner September 9, 2026 14:02
@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 71.31%. Comparing base (fec265c) to head (001bec0).

Files with missing lines Patch % Lines
...ks/labs/lakebridge/connections/database_manager.py 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2658   +/-   ##
=======================================
  Coverage   71.31%   71.31%           
=======================================
  Files         112      112           
  Lines       10122    10125    +3     
  Branches     1111     1111           
=======================================
+ Hits         7218     7221    +3     
  Misses       2690     2690           
  Partials      214      214           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

✅ 182/182 passed, 8 flaky, 2 skipped, 2h25m28s total

Flaky tests:

  • 🤪 test_installs_and_runs_local_bladebridge (13.068s)
  • 🤪 test_installs_and_runs_pypi_bladebridge (24.837s)
  • 🤪 test_transpiles_informatica_to_sparksql_non_interactive[False] (23.846s)
  • 🤪 test_transpiles_informatica_to_sparksql_non_interactive[True] (24.149s)
  • 🤪 test_transpiles_informatica_to_sparksql (25.422s)
  • 🤪 test_transpile_teradata_sql (26.158s)
  • 🤪 test_transpile_teradata_sql_non_interactive[False] (6.65s)
  • 🤪 test_transpile_teradata_sql_non_interactive[True] (9.491s)

Running from acceptance #5452

@gueniai

gueniai commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

The changes LGTM. @surajs-db, can you please address the issues that are making fmt-python fail? Then I think we should be good to migrate

@m-abulazm m-abulazm added the feat/profiler Issues related to profilers label Sep 18, 2026

class MSSQLConnector(DatabaseConnector):
@staticmethod
def odbc_value(value: str) -> str:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename to something more descriptive e.g. escape

Comment thread tests/unit/connections/test_mssql_auth.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat/profiler Issues related to profilers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants