refactor(c/driver_manager): split up adbc_driver_manager.cc for easier maintenance#4029
Merged
zeroshade merged 27 commits intoapache:mainfrom Mar 4, 2026
Merged
Conversation
Extracted driver discovery and loading logic into separate file: - LoadDriverManifest: TOML manifest parsing - ManagedLibrary: Dynamic library management with GetDriverInfo, FindDriver, SearchPathsForDriver, Load, and Lookup methods - GetEnvPaths/GetSearchPaths: Search path resolution - HasExtension: Cross-platform extension checking - Windows Registry integration (RegistryKey class, LoadDriverFromRegistry) - InternalAdbcDriverManagerDefaultEntrypoint: Default entrypoint generation Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Extracted all public ADBC API functions from the monolithic driver manager file into a dedicated API implementation file: - Database API (New, Init, Release, Get/SetOption variants) - Connection API (New, Init, Release, Commit, Rollback, Get/SetOption, GetInfo, GetObjects, etc.) - Statement API (Bind, Execute, Prepare, Get/SetOption, etc.) - Driver loading API (AdbcLoadDriver, AdbcLoadDriverFromInitFunc) - Default stubs for unimplemented driver functions - ErrorArrayStream wrapper for enhanced error reporting - Helper structures (TempDatabase, TempConnection) moved to internal header Updated internal header to include shared structures and function declarations needed across multiple translation units. Updated CMake build to include new source files. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Added inline destructor implementation for OwnedError - Added declarations for AdbcFindLoadDriver and ReleaseDriver functions - Removed static keyword from ReleaseDriver to allow cross-file usage - Removed redundant extern declarations from API file Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fixed multiple categories of compilation and linker errors after splitting the driver manager into separate source files: **Duplicate Definitions:** - Removed duplicate structure definitions (ParseDriverUriResult, DriverInfo, OwnedError) from .cc files; kept only in internal header - Removed duplicate error handling functions (SetError, AppendError) from anonymous namespaces in .cc files - Moved error handling implementations to global namespace in main file - Removed duplicate AdbcStatusCodeMessage (kept in API file) **Namespace Issues:** - Moved enums and type aliases from anonymous namespaces to internal header - Moved utility functions (AddSearchPathsToError) out of anonymous namespace - Created wrapper for ReleaseDriver to expose from anonymous namespace - Moved FilesystemProfile and ProcessProfileValue/LoadProfileFile out of anonymous namespace in profiles file **Visibility and Linkage:** - Added declarations to internal header for shared functions - Removed declarations for functions that should remain file-local - Made CurrentArch() inline to avoid multiple definition errors - Properly scoped helper functions within anonymous namespaces **Unused Code:** - Commented out unused helper functions (GetProfileSearchPaths, GetEnvPaths, kAdbcProfilePath) to avoid unused-function warnings - Removed GetProfileSearchPaths declaration from internal header **Cross-file Dependencies:** - Added include of internal header in profiles file - Moved AdbcErrorFromArrayStream and AdbcErrorGetDetail to API file with ErrorArrayStream helpers - Ensured all split files can access shared utilities Build now completes successfully with no errors or warnings. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ementation The AdbcProfileProviderFilesystem function was not moved to the profiles file during the refactoring. This commit: - Uncomments and activates GetEnvPaths helper function - Uncomments and activates GetProfileSearchPaths helper function - Adds the main AdbcProfileProviderFilesystem function implementation - Adds CHECK_STATUS macro definition needed by the profiles file - Uncomments kAdbcProfilePath constant This fixes the undefined symbol error that was preventing tests from running. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
lidavidm
reviewed
Feb 28, 2026
paleolimbot
reviewed
Mar 2, 2026
Member
paleolimbot
left a comment
paleolimbot
left a comment
There was a problem hiding this comment.
Nice!
To fix R I think you need to add a few lines here:
arrow-adbc/r/adbcdrivermanager/src/Makevars.win
Lines 23 to 29 in f2d58a7
...and here:
arrow-adbc/r/adbcdrivermanager/src/Makevars
Lines 22 to 28 in f2d58a7
Member
Author
|
@lidavidm @paleolimbot I managed to make the fixes suggested and got all the tests passing, finally! Ready for another review pass, thanks! |
lidavidm
approved these changes
Mar 4, 2026
Member
lidavidm
left a comment
lidavidm
left a comment
There was a problem hiding this comment.
I didn't compare the diffs in detail so I'm going to trust that everything was split without modification, but the organization looks fine to me.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As discussed in some of the recent PRs, this refactors the
adbc_driver_manager.ccfile and splits things up into multiple files based on the API surface they are relevant to. This makes future maintenance easier for the C++ driver manager.This also updates the pre-commit config to ensure the files stay in sync with the go/adbc/drivermgr files etc.