feat(csharp): native C# Snowflake ADBC driver (REST-based) - #171
feat(csharp): native C# Snowflake ADBC driver (REST-based)#171ndglover wants to merge 16 commits into
Conversation
6a7ba51 to
aaba4ec
Compare
There was a problem hiding this comment.
I thought certain IDE files used CRLF?
There was a problem hiding this comment.
I found that by not fixing it to one or the other you end up with certain IDEs automatically modifying the files. This means you need to manually handle this in git by reverting these changes or you end up in ding dong between different conventions people have. I believe fixing to one works better but open to other suggestions
|
Thanks @lidavidm for leaving a comment which reminded me of this PR. I'll set some time aside tomorrow evening to look at it. There are a bunch of linter issues around whitespace and at least one file missing a copyright notice. I also wonder whether the copyright year should be 2026 ;). And this is probably just a "me" thing, but I find I keep getting confused by the use of "Native" to refer to the managed implementation (being "native C#"). |
| ArgumentNullException.ThrowIfNull(batch); | ||
| ArgumentNullException.ThrowIfNull(schema); | ||
|
|
||
| _boundParameters = batch; |
There was a problem hiding this comment.
If _boundParameters is already set, should we dispose it?
| if (parameters.TryGetValue("adbc.snowflake.sql.client_option.request_timeout", out string? requestTimeoutStr) && | ||
| int.TryParse(requestTimeoutStr, out int requestTimeoutSeconds)) | ||
| { | ||
| config.QueryTimeout = TimeSpan.FromSeconds(requestTimeoutSeconds); | ||
| } |
There was a problem hiding this comment.
If the value is present but does not parse, presumably we should error?
There was a problem hiding this comment.
Yes, this is a better pattern, thanks. I've applied a fix
Adds a from-scratch, fully-managed C# implementation of the Snowflake ADBC driver (net8.0), independent of the Go/cgo Interop driver. It speaks Snowflake's internal REST protocol directly and returns Arrow natively. Add QueryTag to connection and statement
|
@CurtHagenlocher - I think I've fixed up the linting issues and missing license headers now. |
There was a problem hiding this comment.
Pull request overview
Adds a new, fully-managed native C# Snowflake ADBC driver (net8.0) that speaks Snowflake’s REST protocol directly and returns Arrow results natively, plus a comprehensive unit/integration test suite and related developer tooling.
Changes:
- Introduces the core native driver surface (
SnowflakeDriver,SnowflakeDatabase) and supporting services (authentication, transport, query execution, connection pooling). - Adds extensive offline unit tests for protocol/shape handling and correctness, plus opt-in live integration tests and benchmarks.
- Adds C# workspace tooling/config updates (
.editorconfig, ignore rules) and benchmark runner script.
Reviewed changes
Copilot reviewed 100 out of 101 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| csharp/test/Native/SnowflakeResultArrowStreamTests.cs | Offline tests for Arrow result stream fixups (FIXED/TIME/TIMESTAMP handling). |
| csharp/test/Native/SnowflakeDriverTests.cs | Unit tests for driver open/parameter validation behavior. |
| csharp/test/Native/SnowflakeDatabaseTests.cs | Unit tests for HttpMessageHandler ownership/disposal semantics. |
| csharp/test/Native/SnowflakeConnectionTransactionTests.cs | Unit tests for autocommit/transaction control and pool-safe cleanup. |
| csharp/test/Native/SnowflakeAccountUrlTests.cs | Unit tests for account/base URL building behavior. |
| csharp/test/Native/RequestBuilderTests.cs | Unit tests for query/cancel request body construction. |
| csharp/test/Native/QueryExecutorTests.cs | Unit tests for DML affected-row detection and envelope classification helpers. |
| csharp/test/Native/QueryExecutorInProgressTests.cs | Unit tests for long-running query polling and session-renewal during polling. |
| csharp/test/Native/QueryExecutorHeartbeatTests.cs | Unit tests for heartbeat behavior and renewal fallback. |
| csharp/test/Native/QueryExecutorFaultTests.cs | Unit tests for connection-fault callback rules across failure modes. |
| csharp/test/Native/QueryExecutorEmptyResultTests.cs | Unit tests for zero-row Arrow result handling and schema construction. |
| csharp/test/Native/KeyPairAuthenticatorTests.cs | Unit tests for key-pair JWT auth and key-material resolution/validation. |
| csharp/test/Native/Integration/SessionRenewalTests.cs | Live tests for session renewal and heartbeat endpoints. |
| csharp/test/Native/Integration/IntegrationTestingUtils.cs | Integration test configuration loading + parameter mapping helpers. |
| csharp/test/Native/Integration/IntegrationTestConfiguration.cs | Integration test config model (interop-compatible JSON shape). |
| csharp/test/Native/Integration/ConnectionTests.cs | Live connectivity/lifecycle smoke tests for the native driver. |
| csharp/test/Native/Integration/BenchmarkTests.cs | Live benchmark test (native driver) mirroring interop benchmark shape. |
| csharp/test/Native/ConnectionPool/PooledConnectionTests.cs | Unit tests for pooled token-expiry tracking via injected clock. |
| csharp/test/Native/ChunkedArrowArrayStreamTests.cs | Unit tests for chunk prefetch windowing/backpressure and disposal. |
| csharp/test/Native/BindCases.cs | Central bind-case catalog shared by offline/live bind tests. |
| csharp/test/Native/AdbcDrivers.Snowflake.Native.Tests.csproj | New native driver test project and dependencies. |
| csharp/test/Interop/SnowflakeTestConfiguration.cs | Minor formatting update in interop test configuration model. |
| csharp/test/Interop/BenchmarkTests.cs | Interop benchmark test file (mirrors native benchmark shape). |
| csharp/src/Native/SnowflakeDriver.cs | Native ADBC driver entry point (Open). |
| csharp/src/Native/SnowflakeDatabase.cs | Database implementation (HttpClient creation, pooling, Connect/Dispose). |
| csharp/src/Native/Services/TypeConversion/SnowflakeTypeCode.cs | Snowflake type code enum. |
| csharp/src/Native/Services/TypeConversion/SnowflakeDataType.cs | Snowflake type metadata model + type-name parsing. |
| csharp/src/Native/Services/TypeConversion/ParameterSet.cs | Bind parameter set model for positional parameters. |
| csharp/src/Native/Services/TypeConversion/ITypeConverter.cs | Type conversion interface (Snowflake→Arrow and Arrow→binds). |
| csharp/src/Native/Services/Transport/SnowflakeRequestBodies.cs | Query/cancel/renew request models + binding JSON converter. |
| csharp/src/Native/Services/Transport/SnowflakeProtocolConstants.cs | Protocol constants (session parameter keys/values, bind type names). |
| csharp/src/Native/Services/Transport/SnowflakeJsonContext.cs | Source-generated JSON serializer context for query protocol types. |
| csharp/src/Native/Services/Transport/RequestBuilder.cs | Builder for query and cancel request bodies. |
| csharp/src/Native/Services/Transport/IRestApiClient.cs | REST client abstraction for POST/GET and Arrow chunk streams. |
| csharp/src/Native/Services/Transport/ApiResponse.cs | Generic Snowflake API response envelope model. |
| csharp/src/Native/Services/SnowflakeAccountUrl.cs | Base account URL builder supporting host/protocol/port overrides. |
| csharp/src/Native/Services/Session/SnowflakeSessionClient.cs | Pool-facing session lifecycle implementation (heartbeat/close). |
| csharp/src/Native/Services/Query/SnowflakeQueryResponse.cs | Query response wire model (rowtype/rowset/chunks, result URL). |
| csharp/src/Native/Services/Query/QueryStatus.cs | Query status enum. |
| csharp/src/Native/Services/Query/QueryResult.cs | Query result model (stream, row count, affected rows, errors). |
| csharp/src/Native/Services/Query/QueryRequest.cs | Query request model (statement/session context/binds/timeouts). |
| csharp/src/Native/Services/Query/QueryError.cs | Query error model (code/message/exception). |
| csharp/src/Native/Services/Query/PreparedStatement.cs | Prepared statement model (result schema only). |
| csharp/src/Native/Services/Query/IQueryExecutor.cs | Query execution interface (execute/describe/renew/heartbeat/cancel). |
| csharp/src/Native/Services/Query/EmptyArrowArrayStream.cs | Empty Arrow stream implementation carrying schema only. |
| csharp/src/Native/Services/Query/ChunkedArrowArrayStream.cs | Arrow result streaming over inline+chunked data with prefetching. |
| csharp/src/Native/Services/ConnectionPool/PooledConnection.cs | Pooled connection implementation (token expiry, timestamps, close). |
| csharp/src/Native/Services/ConnectionPool/ISessionLifecycle.cs | Pool callback interface for heartbeat/close over transport layer. |
| csharp/src/Native/Services/ConnectionPool/IPooledConnection.cs | Pooled connection interface (timestamps, token, faulting, lifecycle). |
| csharp/src/Native/Services/ConnectionPool/IConnectionPoolManager.cs | Pool manager interface (acquire/release). |
| csharp/src/Native/Services/ConnectionPool/ConnectionPoolEntry.cs | Pool entry state (active/idle collections, capacity semaphore). |
| csharp/src/Native/Services/Authentication/SnowflakeLoginClient.cs | Login/close-session client using Snowflake login protocol. |
| csharp/src/Native/Services/Authentication/PatAuthenticator.cs | PAT authentication implementation and requirement validation. |
| csharp/src/Native/Services/Authentication/OAuthAuthenticator.cs | OAuth authentication implementation and requirement validation. |
| csharp/src/Native/Services/Authentication/LoginResponseModels.cs | Login/authenticator response models. |
| csharp/src/Native/Services/Authentication/LoginRequestModels.cs | Login request models (web-default JSON naming). |
| csharp/src/Native/Services/Authentication/KeyPairAuthenticator.cs | Key-pair JWT auth implementation + key resolution helpers. |
| csharp/src/Native/Services/Authentication/ISsoAuthenticator.cs | SSO authenticator interface. |
| csharp/src/Native/Services/Authentication/IPatAuthenticator.cs | PAT authenticator interface. |
| csharp/src/Native/Services/Authentication/IOAuthAuthenticator.cs | OAuth authenticator interface. |
| csharp/src/Native/Services/Authentication/IKeyPairAuthenticator.cs | Key-pair authenticator interface. |
| csharp/src/Native/Services/Authentication/IBasicAuthenticator.cs | Username/password authenticator interface. |
| csharp/src/Native/Services/Authentication/IAuthenticationService.cs | Authentication service interface. |
| csharp/src/Native/Services/Authentication/ClientEnvironment.cs | Client environment payload model for login. |
| csharp/src/Native/Services/Authentication/BasicAuthenticator.cs | Username/password auth implementation and requirement validation. |
| csharp/src/Native/Services/Authentication/AuthenticationToken.cs | Token model (session + master validity windows). |
| csharp/src/Native/Services/Authentication/AuthenticationService.cs | Auth dispatcher by configured authentication type. |
| csharp/src/Native/InMemoryArrowStream.cs | In-memory Arrow stream for metadata-style prebuilt batches. |
| csharp/src/Native/Configuration/NetworkConfig.cs | Network config (host/protocol/port/proxy/TLS skip verify). |
| csharp/src/Native/Configuration/ConnectionPoolConfig.cs | Pool sizing/timeouts configuration model. |
| csharp/src/Native/Configuration/ConnectionConfig.cs | Connection config model (auth/network/pool/timeouts/keep-alive). |
| csharp/src/Native/Configuration/AuthenticationType.cs | Authentication type enum. |
| csharp/src/Native/Configuration/AuthenticationConfig.cs | Authentication config model (password/key/token/SSO props). |
| csharp/src/Native/AdbcDrivers.Snowflake.Native.csproj | Native driver project definition and packaging metadata. |
| csharp/run_benchmark.ps1 | Script to run and compare native vs interop benchmarks and summarize stats. |
| csharp/.gitignore | C# workspace ignore updates (IDE artifacts). |
| csharp/.editorconfig | C# workspace EditorConfig (layout rules + silent style hints). |
| .gitignore | Repo-level ignore updates for Go shared library artifacts. |
| .gitattributes | Repo-level text/eol normalization settings. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| var protocol = network?.Protocol ?? "https"; | ||
| var host = account.Contains("snowflakecomputing.com", System.StringComparison.OrdinalIgnoreCase) | ||
| ? account | ||
| : $"{account}.snowflakecomputing.com"; | ||
| var portSuffix = (network != null && network.Port != 443) ? $":{network.Port}" : string.Empty; | ||
| return $"{protocol}://{host}{portSuffix}"; |
There was a problem hiding this comment.
I've made some changes to make it more robust and take into account different region and cloud suffixes
| <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="9.0.0" /> | ||
| <PackageReference Include="Microsoft.Extensions.TimeProvider.Testing" Version="9.0.0" /> |
There was a problem hiding this comment.
I'm leaving this for now, it's minor and will be better fixed when we pull the latest arrow packages with different transitive dependencies
| AuthenticationType.UsernamePassword => auth.Password, | ||
| AuthenticationType.OAuth or AuthenticationType.Pat => auth.Token, | ||
| AuthenticationType.KeyPair => $"{auth.PrivateKey ?? auth.PrivateKeyPath} {auth.PrivateKeyPassphrase}", | ||
| _ => null, |
There was a problem hiding this comment.
Created CredentialFieldDelimiter
CurtHagenlocher
left a comment
There was a problem hiding this comment.
I've taken a pass over all the files and left a random assortment of comments. Looks good overall; I don't think there's anything that critically needs to be changed but I do have some suggestions for things that (I think :) would be improvements.
After the first two hours, I stopped focusing on the Snowflake-specific stuff and focused on the Arrow-specific code where I probably add more value anyway.
| @@ -0,0 +1,64 @@ | |||
| /* | |||
| * Copyright (c) 2025 ADBC Drivers Contributors | |||
There was a problem hiding this comment.
It's closer to 2027 than 2025 ;)
| /// </summary> | ||
| [JsonPropertyName("roleInfo")] | ||
| public RoleInfo? RoleInfo { get; set; } | ||
|
|
There was a problem hiding this comment.
nit: any reason for the added whitespace?
| EndProject | ||
| Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{827E0CD3-B72D-47B6-A68D-7590B98EB39B}" | ||
| EndProject | ||
| Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Native", "Native", "{986E768A-9E42-6229-8E82-349DB5D13BDD}" |
There was a problem hiding this comment.
I find the use of "native" to mean "natively C#" a bit confusing. In the context of .NET it's usually used to mean the opposite of "managed" -- at least in my world; maybe this isn't common?
There was a problem hiding this comment.
See for example https://github.com/adbc-drivers/hiveserver2/tree/main/csharp/src and https://github.com/CurtHagenlocher/bigquery/tree/curth_microsoft/aot-clast-packages/csharp/src (which, to be fair, are me) but also https://github.com/lomakinandrei/NativeWPF and https://github.com/3F/Conari and even https://learn.microsoft.com/en-us/dotnet/core/deploying/native-aot -- not to belabor the point ;).
| return typeName.ToUpperInvariant() switch | ||
| { | ||
| "FIXED" or "NUMBER" or "DECIMAL" or "NUMERIC" => SnowflakeTypeCode.Number, | ||
| "INTEGER" or "INT" or "BIGINT" or "SMALLINT" or "TINYINT" or "BYTEINT" => SnowflakeTypeCode.Integer, |
There was a problem hiding this comment.
Not really a review comment, but I'm curious -- under what circumstances Snowflake will return one of these type names?
There was a problem hiding this comment.
Good spot, I included the DML types which never get returned. I took a closer look at the types and mapping and have made this more robust with added integration test,
| SnowflakeTypeCode.TimestampLtz => new TimestampType(TimeUnit.Nanosecond, timezone: "UTC"), | ||
|
|
||
| // The result decoder stores TIMESTAMP_TZ as its UTC instant (a single Arrow column | ||
| // cannot carry a per-row offset), so the described type matches: Timestamp[ns] "UTC". |
There was a problem hiding this comment.
FWIW, in arrow-dotnet 23 there's an Arrow extension type that stores the rough equivalent of a .NET DateTimeOffset. It's TimestampWithOffsetType.
There was a problem hiding this comment.
Need to upgrade to v23, I'll tackle in subsequent PR
| if (!responseContent.Success) | ||
| { | ||
| var errorMessage = responseContent.Message ?? "Authentication failed."; | ||
| throw new AdbcException($"Snowflake authentication failed: {errorMessage}"); |
There was a problem hiding this comment.
In general, it would be nice to try to assign an AdbcStatusCode to the exceptions. This is particularly true for authentication-related exceptions, which a consumer has no way to distinguish from other kinds of exceptions except by examining the text.
| var first = chunkList[0]; | ||
| chunkList.RemoveAt(0); | ||
| firstStream = await apiClient.GetArrowStreamAsync(first.Url, authToken, chunkHeaders, qrmk, cancellationToken).ConfigureAwait(false); | ||
| firstReader = new Ipc.ArrowStreamReader(firstStream); |
There was a problem hiding this comment.
You should be able to pass a byte[] directly to the constructor of an ArrowStreamReader; it has an overload that takes a ReadOnlyMemory<byte>. This will be more efficient than wrapping the memory in a Stream.
There was a problem hiding this comment.
I'd like to tackle this and some of the other performance related comments when we have updated to v23
| return null; | ||
|
|
||
| _currentStream = nextChunk.Stream; | ||
| _currentReader = new Ipc.ArrowStreamReader(_currentStream); |
There was a problem hiding this comment.
If you know this is going to be a MemoryStream (or even if you suspect it could be) then it might be worth extracting the underlying buffer and converting it to a ReadOnlyMemory of the appropriate length and passing that to the constructor of ArrowStreamReader. (Or at least it's worth benchmarking -- I don't remember exactly what the deserialization tries to do and it's possible that using a Stream as a source gives more but smaller allocations in a way that actually ends up better off by avoiding the LOH and/or freeing more memory sooner.)
There was a problem hiding this comment.
noted, will bucket with 'I'd like to tackle this and some of the other performance related comments when we have updated to v23'
| case TimestampLtzLogicalType: | ||
| case TimestampTzLogicalType: | ||
| { | ||
| // TZ carries a per-row offset field (dropped — we store the UTC instant); NTZ |
There was a problem hiding this comment.
See other comment about the possibility of using the extension type.
There was a problem hiding this comment.
noted, will bucket with 'I'd like to tackle this and some of the other performance related comments when we have updated to v23'
| private const long NanosecondsPerSecond = 1_000_000_000L; | ||
|
|
||
| private readonly Ipc.IArrowArrayStream _inner; | ||
| private readonly List<ColumnTransform> _transforms; |
There was a problem hiding this comment.
I think there are probably some optimization opportunities in here. The C# builders aren't particularly efficient, and I think you could probably use the new Int128 type in the BCL instead of decimal to load Decimal128Array more efficiently. We really need to get a bunch of this kind of functionality into the base C# Arrow libraries :/.
… to update submodule
…for the cleanup path
Adds a from-scratch, fully-managed C# implementation of the Snowflake ADBC driver (net8.0), independent of the Go/cgo Interop driver. It speaks Snowflake's internal REST protocol directly and returns Arrow natively.
As discussed in our slack chat here is a first cut of the driver.
Tagging @lidavidm @ianmcook @davidhcoe @CurtHagenlocher