Skip to content

Conversation

@gargsaumya
Copy link
Contributor

@gargsaumya gargsaumya commented May 20, 2025

ADO Task ID [AB#37539]
This pull request introduces a new Connection class and supporting infrastructure for managing ODBC database connections in the mssql_python project. Key changes include the implementation of the Connection class, the addition of function pointer typedefs for ODBC APIs, and the creation of utility classes for driver management and handle lifecycle management.

New Connection Class Implementation:

ODBC Function Pointer Typedefs and Utilities:

  • mssql_python/pybind/ddbc_bindings.h: Adds typedefs for ODBC API function pointers, logging utilities, and exception helpers. Introduces the DriverLoader singleton for centralized driver loading and the SqlHandle class for RAII-based handle management.

Checklist

  • Tests Passed (if applicable) : All pytests are passing.
  • Docs Updated (if necessary) : Will be done in upcoming PR once the feature is completely implemented.

Copilot AI review requested due to automatic review settings May 20, 2025 10:18
@gargsaumya gargsaumya changed the base branch from main to saumya/connectionclassc++ May 20, 2025 10:18
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

This PR adds a new ODBC Connection class and underlying support for dynamic driver loading and handle management in the mssql_python project.

  • Implements the Connection class for connect/disconnect, transactions, autocommit, and statement allocation
  • Adds ODBC API function-pointer typedefs, a DriverLoader singleton, and an RAII SqlHandle wrapper
  • Centralizes driver loading and pointer resolution in ddbc_bindings.h

Reviewed Changes

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

File Description
mssql_python/pybind/ddbc_bindings.h Added ODBC API typedefs, logging/exception helpers, DriverLoader, SqlHandle
mssql_python/pybind/connection/connection.h Declared the Connection class and its public interface
mssql_python/pybind/connection/connection.cpp Implemented Connection methods (connect/close, transactions, autocommit)
Comments suppressed due to low confidence (1)

mssql_python/pybind/ddbc_bindings.h:4

  • [nitpick] Fix the grammar in this comment: replace "that is file" with "this file".
// INFO|TODO - Note that is file is Windows specific right now. Making it arch agnostic will be

@gargsaumya gargsaumya force-pushed the saumya/conn_implementation branch 2 times, most recently from 05231ba to cd20ea2 Compare May 20, 2025 10:26
@gargsaumya gargsaumya force-pushed the saumya/connectionclassc++ branch from 7727ace to eb7161e Compare May 20, 2025 10:29
@gargsaumya gargsaumya force-pushed the saumya/conn_implementation branch from cd20ea2 to 37f6dcb Compare May 20, 2025 10:36
@microsoft microsoft deleted a comment from Copilot AI May 20, 2025
@microsoft microsoft deleted a comment from Copilot AI May 20, 2025
@microsoft microsoft deleted a comment from Copilot AI May 20, 2025
@gargsaumya gargsaumya changed the title Saumya/conn implementation FEAT: Implementation of C++ Connection Class API May 20, 2025
@gargsaumya gargsaumya force-pushed the saumya/connectionclassc++ branch from daf84f8 to eca18a0 Compare May 20, 2025 12:24
@gargsaumya gargsaumya changed the base branch from saumya/connectionclassc++ to main May 21, 2025 08:13
@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 requested a review from sumitmsft May 21, 2025 10:45
@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/conn_implementation branch 2 times, most recently from 5ecc8fd to 6a077c6 Compare May 29, 2025 07:55
@gargsaumya gargsaumya force-pushed the saumya/conn_implementation branch from 6a077c6 to c8dfd0e Compare May 29, 2025 10:56
bewithgaurav
bewithgaurav previously approved these changes May 29, 2025
sumitmsft
sumitmsft previously approved these changes May 29, 2025
Copy link
Contributor

@sumitmsft sumitmsft left a comment

Choose a reason for hiding this comment

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

All comments have been resolved. Relevant changes made.

* refactor native layer and create reusable components

* add newline

* Delete copy/assign for DriverLoader singleton

* resolve review comments

* initial edit

* working flow with c++ connection class

* working interation with access token

* cleanup free() in sqlhandle

* removed unnecessary prints

* removing comment

* resolving conflict

* working

* final working-fix test

* minor updates

* updating file

* added a TODO comment to address review comments

---------

Co-authored-by: Saumya Garg <gargsaumya@microsoft.com>
@sumitmsft sumitmsft dismissed stale reviews from bewithgaurav and themself via e97b287 May 29, 2025 15:21
Copy link
Collaborator

@bewithgaurav bewithgaurav left a comment

Choose a reason for hiding this comment

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

@gargsaumya gargsaumya merged commit fc99220 into main May 29, 2025
4 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