Skip to content

Comments

FIX: Invalid check for database in connection string in _bulkcopy#445

Open
subrata-ms wants to merge 5 commits intomainfrom
subrata-ms/bulkcopy
Open

FIX: Invalid check for database in connection string in _bulkcopy#445
subrata-ms wants to merge 5 commits intomainfrom
subrata-ms/bulkcopy

Conversation

@subrata-ms
Copy link
Contributor

@subrata-ms subrata-ms commented Feb 24, 2026

AB#42753

GitHub Issue: #442


Summary

This pull request updates the bulk copy functionality to make the DATABASE parameter in the connection string optional, aligning behavior with SQL Server's default handling. It also adds a new test to ensure this works as expected. The most important changes are:

Bulk copy connection string handling:

  • Modified the _bulkcopy method in cursor.py to remove the requirement for the DATABASE parameter in the connection string; now, bulk copy will proceed as long as a server is specified, and the server will use the default database if none is provided.

Testing improvements:

  • Added a new test, test_bulkcopy_without_database_parameter, in test_019_bulkcopy.py to verify that bulk copy works correctly when the DATABASE parameter is omitted, ensuring the client connects to the default database and data is inserted as expected.

@github-actions github-actions bot added the pr-size: small Minimal code update label Feb 24, 2026
@github-actions
Copy link

github-actions bot commented Feb 24, 2026

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

76%


📈 Total Lines Covered: 5520 out of 7185
📁 Project: mssql-python


Diff Coverage

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

No lines with coverage information in this diff.


📋 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.cpp: 69.4%
mssql_python.pybind.ddbc_bindings.h: 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.7%
mssql_python.__init__.py: 84.9%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

@github-actions github-actions bot added pr-size: medium Moderate update size and removed pr-size: small Minimal code update labels Feb 24, 2026
@subrata-ms subrata-ms marked this pull request as ready for review February 24, 2026 07:12
Copilot AI review requested due to automatic review settings February 24, 2026 07:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates bulk copy connection-string validation so DATABASE is no longer required, matching SQL Server’s default-database behavior, and adds tests to validate bulk copy behavior when DATABASE is omitted.

Changes:

  • Remove the _bulkcopy() hard requirement that DATABASE be present in the connection string.
  • Add an integration test validating bulk copy works when DATABASE is omitted.
  • Add tests around SERVER parameter handling (including synonyms and missing-server failure).

Reviewed changes

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

File Description
mssql_python/cursor.py Removes the DATABASE-required guard; keeps a guard ensuring some server key is present before starting py-core bulk copy.
tests/test_019_bulkcopy.py Adds new bulk copy integration tests for missing DATABASE, server-key synonyms, and missing server failure behavior.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-size: medium Moderate update size

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants