Skip to content

Conversation

@jadewang-db
Copy link
Contributor

@jadewang-db jadewang-db commented Oct 14, 2025

This PR introduces a comprehensive design document for adding Databricks Statement Execution API support to the
Databricks C# ADBC driver as an alternative to the current Thrift-based protocol. The design focuses on code
reuse, backward compatibility, and simplification.

@jadewang-db jadewang-db changed the title High level design of adding SEA support for Databricks C# driver feat(csharp/src/Drivers/Databricks): Design of SEA support for Databricks C# driver Oct 16, 2025
@jadewang-db jadewang-db marked this pull request as ready for review October 16, 2025 23:43
@github-actions github-actions bot added this to the ADBC Libraries 21 milestone Oct 16, 2025
Thrift --> TStmt[DatabricksStatement<br/>Thrift]
Thrift --> TCF[CloudFetch<br/>Thrift-based]

REST --> RConn[StatementExecutionConnection]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need separation at this layer? Can't we reuse existing Statement and Connection classes, and reuse the session as well, and just change the underlying client integration client?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

both statement and connection will have different implementations between thrift and SEA, such as how to execute query and how to get metadata.


REST --> RConn[StatementExecutionConnection]
REST --> RClient[StatementExecutionClient]
REST --> RStmt[StatementExecutionStatement]
Copy link
Contributor

Choose a reason for hiding this comment

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

This approach looks to me as prone to lots of redundant code, which can lead to maintenance cost for bug fixing.

@jadewang-db
Copy link
Contributor Author

we can reduce the redundant code by having a common parent class, this can be done during PR.

Copy link
Contributor

@CurtHagenlocher CurtHagenlocher left a comment

Choose a reason for hiding this comment

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

I'm going to check this in as it looks fine to me and if there's any disagreement then iteration can continue against the checked-in design.

@CurtHagenlocher CurtHagenlocher merged commit 4b868ac into apache:main Oct 28, 2025
9 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.

3 participants