Skip to content

feat(csharp): support typed options in connection profiles - #4914

Open
CurtHagenlocher wants to merge 1 commit into
apache:mainfrom
CurtHagenlocher:csharp-typed-profile-options
Open

CurtHagenlocher wants to merge 1 commit into
apache:mainfrom
CurtHagenlocher:csharp-typed-profile-options

Conversation

@CurtHagenlocher

Copy link
Copy Markdown
Contributor

This adds support for typed options in connection profiles. This diverges from the C implementation for backwards compatibility. Otherwise, a profile like

[Options]
adbc.snowflake.sql.client_option.max_retry_count = 3

would fail in AdbcDatabaseInit. See #4913.

Relates to #4751

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

Static review found no blocking issues; the opt-in APIs preserve compatibility and have focused regression coverage.

0 open findings

What changed in this PR

Adds opt-in typed connection-profile options to the C# driver manager while preserving existing string-based behavior for compatibility.

Changes:

  • Adds typed option merging and database-opening APIs.
  • Documents compatibility constraints and adds tests for type preservation, overrides, and validation.
File Description
csharp/​test/​Apache.Arrow.Adbc.Tests/​DriverManager/​TomlConnectionProfileTests.cs Tests typed options and string-based compatibility.
csharp/​test/​Apache.Arrow.Adbc.Tests/​DriverManager/​FakeAdbcDriver.cs Adds a typed-option test driver and database.
csharp/​src/​Apache.Arrow.Adbc/​DriverManager/​AdbcDriverManager.cs Adds and documents opt-in typed profile APIs.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@CurtHagenlocher

Copy link
Copy Markdown
Contributor Author

@davidhcoe, you've thought more about the driver manager than I have; please take a look.

This branch has not been deployed

No deployments
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.

2 participants