Skip to content

Conversation

@gargsaumya
Copy link
Contributor

@gargsaumya gargsaumya commented May 20, 2025

This pull request makes the database connection and cursor handling in the mssql_python module via new C++ Connection class and simplifying the Python-side implementation. The changes include replacing low-level connection management with a higher-level abstraction, removing redundant methods, and delegating key operations to the new C++ backend. Below are the most important changes grouped by theme:

Refactoring Python Connection Management

  • Replaced manual handle allocation (henv, hdbc) and initialization in mssql_python/connection.py with a new Connection class from the C++ backend. The _initializer, _allocate_environment_handle, _allocate_connection_handle, and related methods were removed.
  • Simplified transaction methods (commit, rollback) and connection lifecycle methods (close) by delegating their logic to the C++ Connection class.
  • Updated autocommit handling to use the set_autocommit and get_autocommit methods from the C++ Connection class.

Refactoring Python Cursor Management

  • Removed checks for closed connections in mssql_python/cursor.py and delegated statement handle allocation to the C++ Connection class.
  • Simplified cursor cleanup by removing explicit handle freeing logic.

Integration with Python Bindings

  • Updated ddbc_bindings.cpp to include the new Connection class, enabling its use in the Python layer.

Checklist

  • Tests Passed (if applicable) : Pytests need to be fixed with connection logic in C++.

Copilot AI review requested due to automatic review settings May 20, 2025 13:04
@gargsaumya gargsaumya changed the base branch from main to saumya/conn_implementation May 20, 2025 13:05
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

Adds a new C++ backend for ODBC handle management and switches the Python modules to use it instead of manual handle calls. Major changes include:

  • Introducing ddbc_bindings.h and associated C++ classes (DriverLoader, SqlHandle, Connection) to centralize and RAII-manage ODBC API calls.
  • Updating Python’s connection.py and cursor.py to delegate operations to the new C++ Connection and SqlHandle bindings.
  • Removing legacy Python initialization, handle allocation/free logic, and replacing it with calls into the C++ layer.

Reviewed Changes

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

Show a summary per file
File Description
mssql_python/pybind/ddbc_bindings.h Added ODBC typedefs, function pointers, DriverLoader, SqlHandle
mssql_python/pybind/connection/connection.h Defined C++ Connection class and methods
mssql_python/pybind/connection/connection.cpp Implemented C++ Connection member functions
mssql_python/cursor.py Removed manual handle allocation/free; uses C++ bindings
mssql_python/connection.py Replaced Python handle management with C++ Connection
Comments suppressed due to low confidence (4)

mssql_python/pybind/ddbc_bindings.h:4

  • Typo in comment: 'is file' should be 'this file'.
// INFO|TODO - Note that is file is Windows specific right now. Making it arch agnostic will be

mssql_python/pybind/connection/connection.h:28

  • [nitpick] The member _conn is unused within the C++ Connection class and creates a circular reference; consider removing it.
    std::shared_ptr<Connection> _conn; 

mssql_python/connection.py:168

  • self._conn.commit() calls a method that isn't defined in the C++ Connection class. Implement a commit() wrapper (calling end_transaction(SQL_COMMIT)) or update Python to use end_transaction directly.
        self._conn.commit()

mssql_python/connection.py:184

  • self._conn.rollback() references a non-existent C++ method. Provide a rollback() wrapper (calling end_transaction(SQL_ROLLBACK)) or adjust Python to use the end_transaction API.
        self._conn.rollback()

@gargsaumya gargsaumya changed the title Saumya/integratec++class FEAT: Integrate python <->C++ flow with C++ connection class May 20, 2025
@gargsaumya gargsaumya force-pushed the saumya/integratec++class branch 2 times, most recently from 119f8ce to 9813c41 Compare May 20, 2025 14:00
@microsoft microsoft deleted a comment from Copilot AI May 20, 2025
@microsoft microsoft deleted a comment from Copilot AI May 20, 2025
@gargsaumya gargsaumya force-pushed the saumya/conn_implementation branch 3 times, most recently from 56cf353 to c89cfbe Compare May 21, 2025 09:12
@gargsaumya gargsaumya force-pushed the saumya/integratec++class branch from 9813c41 to 657b68b Compare May 21, 2025 12:25
@gargsaumya gargsaumya force-pushed the saumya/conn_implementation branch from f979de7 to 896f34d Compare May 21, 2025 14:23
@gargsaumya gargsaumya force-pushed the saumya/integratec++class branch 2 times, most recently from 3c7124b to 87257e7 Compare May 21, 2025 15:32
@gargsaumya gargsaumya force-pushed the saumya/integratec++class branch from 87257e7 to b22b197 Compare May 21, 2025 15:37
@gargsaumya gargsaumya force-pushed the saumya/conn_implementation branch from 5ecc8fd to 6a077c6 Compare May 29, 2025 07:55
@gargsaumya gargsaumya force-pushed the saumya/integratec++class branch from 62f23ed to 3240394 Compare May 29, 2025 10:42
@gargsaumya gargsaumya force-pushed the saumya/conn_implementation branch from 6a077c6 to c8dfd0e Compare May 29, 2025 10:56
@gargsaumya gargsaumya force-pushed the saumya/integratec++class branch from fa7a9fa to a9ee888 Compare May 29, 2025 11:05
@sumitmsft sumitmsft merged commit e97b287 into saumya/conn_implementation May 29, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants