- Notifications
You must be signed in to change notification settings - Fork11
FEAT: Bulk Copy Wrapper#108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:jahnvi/bulk_copy_main_apis
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There is no explicit thread safety; if the same BCPWrapper object is used from multiple threads, state flags and ODBC calls could race.
Some methods (e.g., bcp_control int overload) throw exceptions on error, others (e.g., bcp_control wstring overload, read_format_file) just return SQL_ERROR. For a pybind11-exposed wrapper, it's better to throw exceptions for all error states, so Python users get errors, not silent failures.
Recommendation:
Add a note/documentation about thread-safety, or add a mutex if needed.
Always throw std::runtime_error on unrecoverable errors.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
…soft/mssql-python into jahnvi/bulk_copy_wrapper_cpp
| // Handle binary data | ||
| std::string binValue = data.cast<std::string>(); | ||
| auto buffer = std::shared_ptr<unsigned char[]>(new unsigned char[binValue.length()]); | ||
| memcpy(buffer.get(), binValue.data(), binValue.length()); |
Check notice
Code scanning / devskim
There are a number of conditions in which memcpy can introduce a vulnerability (mismatched buffer sizes, null pointers, etc.). More secure alternitives perform additional validation of the source and destination buffer Note
Uh oh!
There was an error while loading.Please reload this page.
ADO Work Item Reference
Summary
This pull request introduces significant enhancements to the MSSQL Python library by integrating Bulk Copy Program (BCP) functionality. This includes the creation of a new
BCPWrapperclass, updates to the connection handling, and modifications to the build system to support the new functionality. Below is a summary of the most important changes:BCP Integration
BCPWrapperclass inmssql_python/pybind/bcp/bcp_wrapper.hto encapsulate BCP operations, including methods for initialization, column format definition, data binding, and execution. The class also ensures proper resource cleanup.BCPWrapperto Python viapybind11inmssql_python/pybind/ddbc_bindings.cpp, allowing Python users to perform bulk copy operations programmatically.Connection Enhancements
ConnectionandConnectionHandleclasses inmssql_python/pybind/connection/connection.hto provide access to the database connection handle (getDbcHandle) and the associatedConnectionobject (getConnection). These changes support the integration of BCP functionality.[1][2]Connection::applyAttrsBeforeinmssql_python/pybind/connection/connection.cppto handle theSQL_COPT_SS_BCPattribute, enabling BCP operations before connecting.Build System Updates
CMakeLists.txtinmssql_python/pybindto include the newbcp_wrapper.cppfile and its header directory, ensuring proper compilation of the BCP functionality.[1][2]Driver and API Integration
bcp_initW,bcp_control,bcp_exec) inmssql_python/pybind/ddbc_bindings.handmssql_python/pybind/ddbc_bindings.cpp. These APIs are dynamically loaded from the ODBC driver during runtime.[1][2]LoadDriverOrThrowExceptionto include BCP function pointers, ensuring they are available for use.Code Cleanup and Minor Fixes
self.wrapper.finish()call inmssql_python/bcp_main.pyas the new BCPWrapper class handles resource cleanup internally.SQLGetData_wrapto accurately describe the conversion of timestamp fractions.