diff --git a/csharp/src/DatabricksDatabase.cs b/csharp/src/DatabricksDatabase.cs index 012ddd633..1ced01b0d 100644 --- a/csharp/src/DatabricksDatabase.cs +++ b/csharp/src/DatabricksDatabase.cs @@ -24,6 +24,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text.RegularExpressions; using AdbcDrivers.Databricks.StatementExecution; using Apache.Arrow.Adbc; using AdbcDrivers.HiveServer2; @@ -79,12 +80,10 @@ public override AdbcConnection Connect(IReadOnlyDictionary? opti // Merge with environment config (DATABRICKS_CONFIG_FILE) and feature flags from server mergedProperties = MergeWithEnvironmentConfigAndFeatureFlags(mergedProperties); - // Check protocol selection - string protocol = "thrift"; // default - if (mergedProperties.TryGetValue(DatabricksParameters.Protocol, out var protocolValue)) - { - protocol = protocolValue.ToLowerInvariant(); - } + // Resolve protocol via explicit override -> httpPath auto-detection -> legacy default. + // PECO-3055: matches JDBC's compute-resource-based protocol selection + // (DatabricksConnectionContext.getClientTypeFromContext). + string protocol = ResolveProtocol(mergedProperties); AdbcConnection connection; @@ -95,26 +94,38 @@ public override AdbcConnection Connect(IReadOnlyDictionary? opti // including TracingDelegatingHandler, RetryHttpHandler, and OAuth authentication // handlers (OAuthDelegatingHandler, TokenRefreshDelegatingHandler, // MandatoryTokenExchangeDelegatingHandler) when OAuth auth is configured - connection = new StatementExecutionConnection( - mergedProperties, - this.RecyclableMemoryStreamManager, - this.Lz4BufferPool); - - // Open the connection to create session if needed - var statementConnection = (StatementExecutionConnection)connection; - statementConnection.OpenAsync().Wait(); + bool fallbackToThrift = false; + try + { + connection = new StatementExecutionConnection( + mergedProperties, + this.RecyclableMemoryStreamManager, + this.Lz4BufferPool); + + // Open the connection to create session if needed + var statementConnection = (StatementExecutionConnection)connection; + statementConnection.OpenAsync().Wait(); + } + catch (Exception ex) when (!IsExplicitProtocolOverride(mergedProperties) && IsTemporaryRedirectError(ex)) + { + // PECO-3055: SEA -> Thrift fallback on HTTP 307 from createSession. + // The redirect signals the workspace routes SEA traffic back to Thrift + // (e.g., region-locked SEA disable). Mirrors JDBC's + // DatabricksSession.open() handling of DatabricksTemporaryRedirectException. + // Only fall back when protocol was auto-detected — explicit "rest" + // selection from the user must surface the error. + fallbackToThrift = true; + connection = null!; + } + + if (fallbackToThrift) + { + connection = OpenThriftConnection(mergedProperties); + } } else if (protocol == "thrift") { - // Use traditional Thrift/HiveServer2 protocol - connection = new DatabricksConnection( - mergedProperties, - this.RecyclableMemoryStreamManager, - this.Lz4BufferPool); - - var databricksConnection = (DatabricksConnection)connection; - databricksConnection.OpenAsync().Wait(); - databricksConnection.ApplyServerSidePropertiesAsync().Wait(); + connection = OpenThriftConnection(mergedProperties); } else { @@ -248,5 +259,153 @@ private static IReadOnlyDictionary MergeProperties(IReadOnlyDict return merged; } + + // PECO-3055: regexes mirror the JDBC driver's path patterns + // (DatabricksJdbcConstants.HTTP_WAREHOUSE_PATH_PATTERN / HTTP_ENDPOINT_PATH_PATTERN / + // HTTP_CLUSTER_PATH_PATTERN). Kept here so the protocol decision logic is co-located + // with the only consumer; the SEA-side warehouse-ID extraction in + // StatementExecutionConnection uses its own stricter regex for /sql/1.0/(...)/{id}. + private static readonly Regex s_warehousePathPattern = + new Regex(@".*/warehouses/.+", RegexOptions.Compiled); + private static readonly Regex s_endpointPathPattern = + new Regex(@".*/endpoints/.+", RegexOptions.Compiled); + private static readonly Regex s_clusterPathPattern = + new Regex(@".*/o/.+/.+", RegexOptions.Compiled); + + /// + /// PECO-3055: Resolves the protocol ("rest" for SEA, "thrift" for HiveServer2) using the same + /// priority order as JDBC's DatabricksConnectionContext.getClientTypeFromContext: + /// + /// If is set explicitly, honor it. + /// Otherwise inspect the httpPath: + /// + /// Matches .../o/.+/.+ (general-purpose cluster) → force thrift. + /// Matches .../warehouses/.+ or .../endpoints/.+ → default to rest. + /// + /// + /// Otherwise fall back to thrift (preserves pre-3055 behavior for + /// unparseable paths and unit-test fixtures that omit httpPath). + /// + /// + internal static string ResolveProtocol(IReadOnlyDictionary properties) + { + if (properties.TryGetValue(DatabricksParameters.Protocol, out var explicitProtocol) + && !string.IsNullOrWhiteSpace(explicitProtocol)) + { + return explicitProtocol.ToLowerInvariant(); + } + + string? httpPath = GetHttpPath(properties); + if (!string.IsNullOrEmpty(httpPath)) + { + // Strip query string before matching so /sql/1.0/warehouses/abc?o=123 still + // resolves as a warehouse path. + string pathOnly = httpPath!; + int q = pathOnly.IndexOf('?'); + if (q >= 0) pathOnly = pathOnly.Substring(0, q); + + // GP cluster wins over warehouse/endpoint when both could match + // (cluster paths never contain "/warehouses/" in practice, but JDBC checks + // warehouse first then cluster; we keep the cluster check first to make the + // forced-Thrift rule explicit per the PECO-3055 ticket description). + if (s_clusterPathPattern.IsMatch(pathOnly)) + { + return "thrift"; + } + if (s_warehousePathPattern.IsMatch(pathOnly) || s_endpointPathPattern.IsMatch(pathOnly)) + { + return "rest"; + } + } + + // Fallback: legacy default. Preserves backwards compatibility for callers that + // construct the driver without an httpPath (some integration test harnesses do this). + return "thrift"; + } + + /// + /// Returns true iff the caller passed explicitly. + /// Used to gate the SEA→Thrift 307 fallback: explicit "rest" must surface 307 errors so + /// callers see a clear signal that SEA isn't available for their workspace. + /// + private static bool IsExplicitProtocolOverride(IReadOnlyDictionary properties) => + properties.TryGetValue(DatabricksParameters.Protocol, out var v) && !string.IsNullOrWhiteSpace(v); + + /// + /// Pulls the httpPath property in the same order the rest of the driver does + /// ( first, falling back to 's + /// AbsolutePath). Mirrors StatementExecutionConnection's ctor logic so protocol + /// detection and SEA's warehouse-ID parsing always see the same path. + /// + private static string? GetHttpPath(IReadOnlyDictionary properties) + { + if (properties.TryGetValue(SparkParameters.Path, out var path) && !string.IsNullOrEmpty(path)) + { + return path; + } + if (properties.TryGetValue(AdbcOptions.Uri, out var uri) && !string.IsNullOrEmpty(uri) + && Uri.TryCreate(uri, UriKind.Absolute, out var parsedUri)) + { + return parsedUri.AbsolutePath; + } + return null; + } + + /// + /// Detects whether an exception chain originates from an HTTP 307 (TemporaryRedirect) + /// response — the trigger for SEA → Thrift fallback per PECO-3055. + /// + /// + /// + /// SEA's StatementExecutionClient.EnsureSuccessStatusCodeAsync currently throws + /// with a message containing the literal HTTP status + /// code, so we match on "status code 307". We also recognize the raw + /// with HTTP 307 in its message for + /// the case where the redirect is surfaced before the SEA client wraps it. + /// + /// + /// JDBC uses a dedicated DatabricksTemporaryRedirectException subclass for this; + /// the C# driver doesn't carry HTTP status on its exception type yet, so we use + /// message inspection here. A follow-up could promote this to a typed exception once + /// SEA error mapping is refactored — kept out of scope for PECO-3055. + /// + /// + private static bool IsTemporaryRedirectError(Exception ex) + { + for (Exception? current = ex; current != null; current = current.InnerException) + { + if (current.Message.IndexOf("status code 307", StringComparison.OrdinalIgnoreCase) >= 0) + { + return true; + } + if (current is System.Net.Http.HttpRequestException hre) + { +#if NET5_0_OR_GREATER + if (hre.StatusCode == System.Net.HttpStatusCode.TemporaryRedirect) + { + return true; + } +#endif + } + } + return false; + } + + /// + /// Constructs and opens a Thrift/HiveServer2 . + /// Extracted so both the primary protocol="thrift" branch and the SEA→Thrift 307 + /// fallback path can share the same open sequence (open + ApplyServerSidePropertiesAsync). + /// + private DatabricksConnection OpenThriftConnection(IReadOnlyDictionary mergedProperties) + { + var connection = new DatabricksConnection( + mergedProperties, + this.RecyclableMemoryStreamManager, + this.Lz4BufferPool); + + connection.OpenAsync().Wait(); + connection.ApplyServerSidePropertiesAsync().Wait(); + return connection; + } } } diff --git a/csharp/test/E2E/ClientTests.cs b/csharp/test/E2E/ClientTests.cs index ea2f441ce..7cc6a37a5 100644 --- a/csharp/test/E2E/ClientTests.cs +++ b/csharp/test/E2E/ClientTests.cs @@ -61,14 +61,14 @@ public override void CanClientExecuteQuery() // TODO: PECO-3009 - SEA ADO.NET schema collection calls fail for StatementExecutionConnection public override void VerifySchemaTablesWithNoConstraints() { - Skip.If(TestConfiguration.Protocol == "rest", "SEA ADO.NET schema collection not yet supported (PECO-3009)"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA ADO.NET schema collection not yet supported (PECO-3009)"); base.VerifySchemaTablesWithNoConstraints(); } // TODO: PECO-3009 - SEA ADO.NET schema collection calls fail for StatementExecutionConnection public override void VerifySchemaTables() { - Skip.If(TestConfiguration.Protocol == "rest", "SEA ADO.NET schema collection not yet supported (PECO-3009)"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA ADO.NET schema collection not yet supported (PECO-3009)"); base.VerifySchemaTables(); } diff --git a/csharp/test/E2E/ComplexTypesValueTests.cs b/csharp/test/E2E/ComplexTypesValueTests.cs index 42ea7032d..b0a27eaf7 100644 --- a/csharp/test/E2E/ComplexTypesValueTests.cs +++ b/csharp/test/E2E/ComplexTypesValueTests.cs @@ -97,14 +97,14 @@ private async Task ValidateNullComplexColumnAsync(string sql) // TODO: PECO-3014 - SEA returns NUMERIC/DOUBLE/DATE/TIMESTAMP/INTERVAL array elements in different format protected override async System.Threading.Tasks.Task ValidateTestArrayData(string projection, string value) { - Skip.If(TestConfiguration.Protocol == "rest", "SEA returns array elements in different format for NUMERIC/DOUBLE/DATE/TIMESTAMP/INTERVAL (PECO-3014)"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA returns array elements in different format for NUMERIC/DOUBLE/DATE/TIMESTAMP/INTERVAL (PECO-3014)"); await base.ValidateTestArrayData(projection, value); } // TODO: PECO-3014 - SEA returns map values in different format protected override async System.Threading.Tasks.Task ValidateTestMapData(string projection, string value) { - Skip.If(TestConfiguration.Protocol == "rest", "SEA returns map values in different format (PECO-3014)"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA returns map values in different format (PECO-3014)"); await base.ValidateTestMapData(projection, value); } diff --git a/csharp/test/E2E/DatabricksConnectionTest.cs b/csharp/test/E2E/DatabricksConnectionTest.cs index eb1c8d037..9ea9353c7 100644 --- a/csharp/test/E2E/DatabricksConnectionTest.cs +++ b/csharp/test/E2E/DatabricksConnectionTest.cs @@ -410,7 +410,7 @@ public InvalidConnectionParametersTestData() [SkippableFact] internal void DefaultNamespaceStoredInConnection() { - Skip.If(TestConfiguration.Protocol == "rest", "SEA uses StatementExecutionConnection, not DatabricksConnection (PECO-3009)"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA uses StatementExecutionConnection, not DatabricksConnection (PECO-3009)"); // Skip if default catalog or schema is not configured Skip.If(string.IsNullOrEmpty(TestConfiguration.Catalog), "Default catalog not configured"); Skip.If(string.IsNullOrEmpty(TestConfiguration.DbSchema), "Default schema not configured"); @@ -519,7 +519,7 @@ public async Task SetDefaultCatalogAndSchemaOptionsTest(string? inputCatalog, st // TODO: PECO-3009 - Assert.IsType fails for SEA's StatementExecutionConnection public void TracePropagationConfigurationTest(string tracePropagationEnabled, string traceParentHeaderName, string traceStateEnabled) { - Skip.If(TestConfiguration.Protocol == "rest", "SEA uses StatementExecutionConnection, not DatabricksConnection (PECO-3009)"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA uses StatementExecutionConnection, not DatabricksConnection (PECO-3009)"); // Arrange var testConfig = (DatabricksTestConfiguration)TestConfiguration.Clone(); testConfig.TracePropagationEnabled = tracePropagationEnabled; @@ -546,7 +546,7 @@ public void TracePropagationConfigurationTest(string tracePropagationEnabled, st [SkippableFact] public void TrySetGetDirectResults_UsesDatabricksDefaultGetDirectResults() { - Skip.If(TestConfiguration.Protocol == "rest", "DirectResults is Thrift-only"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "DirectResults is Thrift-only"); var testConfig = (DatabricksTestConfiguration)TestConfiguration.Clone(); using var connection = NewConnection(testConfig); // Create a mock request object diff --git a/csharp/test/E2E/DatabricksTestEnvironment.cs b/csharp/test/E2E/DatabricksTestEnvironment.cs index ccb18b571..807124a6c 100644 --- a/csharp/test/E2E/DatabricksTestEnvironment.cs +++ b/csharp/test/E2E/DatabricksTestEnvironment.cs @@ -59,6 +59,25 @@ private DatabricksTestEnvironment(Func getConnection) : base(get public override string SqlDataResourceLocation => "Resources/Databricks.sql"; + // PECO-3055: protocol now auto-detects from httpPath when not set explicitly. + // Existing tests that assert Thrift-specific behavior key off this helper instead + // of comparing TestConfiguration.Protocol to the literal "rest", which only catches + // the explicit-set case and misses the new "empty Protocol + warehouse path -> SEA" + // default. + public static bool IsResolvedProtocolRest(DatabricksTestConfiguration testConfiguration) + { + // Build the same parameter shape DatabricksTestEnvironment.GetDriverParameters does, + // but only for the fields ResolveProtocol cares about (Protocol, Path, Uri). + var props = new Dictionary(); + if (!string.IsNullOrEmpty(testConfiguration.Protocol)) + props[DatabricksParameters.Protocol] = testConfiguration.Protocol!; + if (!string.IsNullOrEmpty(testConfiguration.Path)) + props[SparkParameters.Path] = testConfiguration.Path!; + if (!string.IsNullOrEmpty(testConfiguration.Uri)) + props[AdbcOptions.Uri] = testConfiguration.Uri!; + return DatabricksDatabase.ResolveProtocol(props) == "rest"; + } + public override int ExpectedColumnCount => 19; public override AdbcDriver CreateNewDriver() => new DatabricksDriver(); diff --git a/csharp/test/E2E/DateTimeValueTests.cs b/csharp/test/E2E/DateTimeValueTests.cs index d983e868c..a60141aad 100644 --- a/csharp/test/E2E/DateTimeValueTests.cs +++ b/csharp/test/E2E/DateTimeValueTests.cs @@ -42,7 +42,7 @@ public DateTimeValueTests(ITestOutputHelper output) public async Task TestTimestampDataDatabricks(DateTimeOffset value, string columnType) { // TODO: PECO-3005 - CommonTestEnvironment.GetValueForProtocolVersion hard-casts to HiveServer2Connection - Skip.If(TestConfiguration.Protocol == "rest", "SEA: GetValueForProtocolVersion hard-casts to HiveServer2Connection"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA: GetValueForProtocolVersion hard-casts to HiveServer2Connection"); await base.TestTimestampData(value, columnType); } @@ -104,7 +104,7 @@ await ValidateInsertSelectDeleteSingleValueAsync( [InlineData("INTERVAL -106751991 DAYS 23 HOURS 59 MINUTES 59.999999 SECONDS", "-106751990 00:00:00.000001000")] public async Task TestIntervalData(string intervalClause, string value) { - Skip.If(TestConfiguration.Protocol == "rest", "SEA returns native Arrow interval types, not String"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA returns native Arrow interval types, not String"); string selectStatement = $"SELECT {intervalClause} AS INTERVAL_VALUE;"; await SelectAndValidateValuesAsync(selectStatement, value, 1); } @@ -115,7 +115,7 @@ public async Task TestIntervalData(string intervalClause, string value) public override Task TestTimestampData(DateTimeOffset value, string columnType) { // TODO: PECO-3005 - CommonTestEnvironment.GetValueForProtocolVersion hard-casts to HiveServer2Connection - Skip.If(TestConfiguration.Protocol == "rest", "SEA: GetValueForProtocolVersion hard-casts to HiveServer2Connection"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA: GetValueForProtocolVersion hard-casts to HiveServer2Connection"); return base.TestTimestampData(value, columnType); } diff --git a/csharp/test/E2E/DriverTests.cs b/csharp/test/E2E/DriverTests.cs index fdb704758..2c077fe62 100644 --- a/csharp/test/E2E/DriverTests.cs +++ b/csharp/test/E2E/DriverTests.cs @@ -100,7 +100,7 @@ protected override void ValidateCanExecuteQuery(double? batchSizeFactor) [SkippableFact, Order(6)] public override void CanGetObjectsAll() { - Skip.If(TestConfiguration.Protocol == "rest", "SEA returns different XdbcColumnSize metadata values (PECO-3005)"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA returns different XdbcColumnSize metadata values (PECO-3005)"); base.CanGetObjectsAll(); } diff --git a/csharp/test/E2E/NumericValueTests.cs b/csharp/test/E2E/NumericValueTests.cs index 0563c2ab8..b00bcb277 100644 --- a/csharp/test/E2E/NumericValueTests.cs +++ b/csharp/test/E2E/NumericValueTests.cs @@ -66,7 +66,7 @@ public override async Task TestDoubleValuesInsertSelectDelete(double value) public override async Task TestFloatValuesInsertSelectDelete(float value) { // TODO: PECO-3005 - CommonTestEnvironment.GetValueForProtocolVersion hard-casts to HiveServer2Connection - Skip.If(TestConfiguration.Protocol == "rest", "SEA: GetValueForProtocolVersion hard-casts to HiveServer2Connection"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA: GetValueForProtocolVersion hard-casts to HiveServer2Connection"); await base.TestFloatValuesInsertSelectDelete(value); } diff --git a/csharp/test/E2E/ProtocolAutoDetectionE2ETests.cs b/csharp/test/E2E/ProtocolAutoDetectionE2ETests.cs new file mode 100644 index 000000000..ab404049a --- /dev/null +++ b/csharp/test/E2E/ProtocolAutoDetectionE2ETests.cs @@ -0,0 +1,169 @@ +/* +* Copyright (c) 2025 ADBC Drivers Contributors +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +using System.Collections.Generic; +using AdbcDrivers.Databricks.StatementExecution; +using AdbcDrivers.HiveServer2.Spark; +using Apache.Arrow.Adbc; +using Apache.Arrow.Adbc.Tests; +using Xunit; +using Xunit.Abstractions; + +namespace AdbcDrivers.Databricks.Tests +{ + /// + /// E2E tests for PECO-3055 httpPath-based protocol auto-detection. + /// Verifies that DatabricksDatabase.Connect picks the right protocol based on + /// the httpPath when adbc.databricks.protocol is not explicitly set. + /// Matches the JDBC driver's compute-type-based protocol selection + /// (DatabricksConnectionContext.getClientTypeFromContext). + /// + public class ProtocolAutoDetectionE2ETests : TestBase + { + public ProtocolAutoDetectionE2ETests(ITestOutputHelper? outputHelper) + : base(outputHelper, new DatabricksTestEnvironment.Factory()) + { + Skip.IfNot(Utils.CanExecuteTestConfig(TestConfigVariable)); + } + + /// + /// When httpPath matches the SQL Warehouse pattern (/sql/1.0/warehouses/{id}) + /// and adbc.databricks.protocol is not provided, the driver must default to + /// the SEA (REST / Statement Execution API) protocol — matching JDBC. + /// + [SkippableFact] + public void WarehousePath_DefaultsToSeaProtocol_WhenProtocolNotSpecified() + { + // Arrange: clone config, clear the explicit protocol so we exercise auto-detection. + var testConfig = (DatabricksTestConfiguration)TestConfiguration.Clone(); + testConfig.Protocol = string.Empty; + + // The shared test warehouse uses /sql/1.0/warehouses/{id} which JDBC maps + // to a Warehouse compute resource → SEA default. Skip if some other path is configured. + Skip.If( + string.IsNullOrEmpty(testConfig.Path) || !testConfig.Path.Contains("/warehouses/"), + "Test requires a warehouse httpPath (/warehouses/{id}) in the test config"); + + // Act: open the connection with no explicit protocol. + using var connection = NewConnection(testConfig); + + // Assert: SEA is the StatementExecutionConnection; Thrift is DatabricksConnection. + Assert.NotNull(connection); + Assert.IsType(connection); + OutputHelper?.WriteLine( + $"Connection type: {connection.GetType().Name} for path '{testConfig.Path}' (no explicit protocol)"); + } + + /// + /// Explicit adbc.databricks.protocol = "thrift" must continue to override the + /// httpPath-based default, even on a warehouse path. This protects existing users + /// who pin to Thrift. + /// + [SkippableFact] + public void ExplicitThriftOverride_BeatsAutoDetection_OnWarehousePath() + { + var testConfig = (DatabricksTestConfiguration)TestConfiguration.Clone(); + testConfig.Protocol = "thrift"; + + Skip.If( + string.IsNullOrEmpty(testConfig.Path) || !testConfig.Path.Contains("/warehouses/"), + "Test requires a warehouse httpPath (/warehouses/{id}) in the test config"); + + using var connection = NewConnection(testConfig); + + Assert.NotNull(connection); + Assert.IsType(connection); + OutputHelper?.WriteLine( + $"Connection type: {connection.GetType().Name} for path '{testConfig.Path}' (protocol=thrift override)"); + } + + /// + /// Explicit adbc.databricks.protocol = "rest" must continue to select SEA, + /// even though that is now the default. This guards against accidental + /// regression of the explicit-override path. + /// + [SkippableFact] + public void ExplicitRestOverride_StillSelectsSea_OnWarehousePath() + { + var testConfig = (DatabricksTestConfiguration)TestConfiguration.Clone(); + testConfig.Protocol = "rest"; + + Skip.If( + string.IsNullOrEmpty(testConfig.Path) || !testConfig.Path.Contains("/warehouses/"), + "Test requires a warehouse httpPath (/warehouses/{id}) in the test config"); + + using var connection = NewConnection(testConfig); + + Assert.NotNull(connection); + Assert.IsType(connection); + } + + /// + /// Unit-style assertion that does not require warehouse round-trip: build the + /// merged properties manually for a GP-cluster httpPath and verify the resolved + /// protocol is "thrift" (forced by compute type). This pins the regex semantics + /// even on a test config that uses a warehouse path. + /// + [SkippableFact] + public void GeneralPurposeClusterPath_ForcesThrift_NoExplicitProtocol() + { + // We use the auto-detection helper indirectly by constructing a DatabricksDatabase + // with cluster-style properties and observing the failure mode of SEA-only path. + // For GP cluster paths (/sql/protocolv1/o/{orgId}/{clusterId}) the driver must + // not try to open a SEA session (which would reject the path). Connecting will + // fail because we don't have a real GP cluster available, but the failure must + // come from the Thrift path — proving we did NOT pick SEA. The cheapest signal + // is that DatabricksDatabase.Connect for a clearly fake host throws an exception + // whose stack/inner-exception originates in the Thrift HiveServer2 transport, + // not in StatementExecutionConnection. + // + // Rather than trying to read the stack trace, we make the inverse assertion: + // if auto-detection were broken (defaulting to SEA on a cluster path), the + // resulting StatementExecutionConnection ctor would throw + // "Statement Execution API requires a SQL Warehouse, not a general cluster". + // We assert that this specific exception is NOT thrown. + var properties = new Dictionary + { + [SparkParameters.HostName] = "example.invalid", + [SparkParameters.Path] = "/sql/protocolv1/o/1234567890/0123-abcdef-foo", + [SparkParameters.Token] = "dummy-token-for-detection-only", + [SparkParameters.AuthType] = "token", + [SparkParameters.Type] = "databricks", + }; + + var driver = NewDriver; + using var database = driver.Open(properties); + + // Either succeeds (impossible here) or throws — but the exception MUST NOT contain + // the SEA-only path rejection, which would mean we incorrectly picked SEA. + try + { + using var connection = database.Connect(properties); + // Connection succeeded against an invalid host — extremely unlikely; if it does + // happen, just dispose and pass: protocol selection clearly didn't go through SEA. + } + catch (System.Exception ex) + { + var flat = ex.ToString(); + Assert.DoesNotContain( + "Statement Execution API requires a SQL Warehouse, not a general cluster", + flat); + OutputHelper?.WriteLine( + $"GP cluster path correctly avoided SEA path. Error chain (truncated): {flat.Substring(0, System.Math.Min(flat.Length, 300))}"); + } + } + } +} diff --git a/csharp/test/E2E/StatementTests.cs b/csharp/test/E2E/StatementTests.cs index a2b56cb46..5c05db047 100644 --- a/csharp/test/E2E/StatementTests.cs +++ b/csharp/test/E2E/StatementTests.cs @@ -48,21 +48,21 @@ public StatementTests(ITestOutputHelper? outputHelper) // TODO: PECO-3011 - SEA StatementExecutionStatement does not validate poll time option protected override void ValidateCanSetOptionPollTime(string value, bool throws = false) { - Skip.If(TestConfiguration.Protocol == "rest", "SEA does not enforce poll time validation"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA does not enforce poll time validation"); base.ValidateCanSetOptionPollTime(value, throws); } // TODO: PECO-3011 - SEA StatementExecutionStatement does not validate query timeout option protected override void ValidateCanSetOptionQueryTimeout(string value, bool throws = false) { - Skip.If(TestConfiguration.Protocol == "rest", "SEA does not enforce query timeout validation"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA does not enforce query timeout validation"); base.ValidateCanSetOptionQueryTimeout(value, throws); } // TODO: PECO-3011 - SEA StatementExecutionStatement does not validate batch size option protected override void ValidateCanSetOptionBatchSize(string value, bool throws = false) { - Skip.If(TestConfiguration.Protocol == "rest", "SEA does not enforce batch size validation"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA does not enforce batch size validation"); base.ValidateCanSetOptionBatchSize(value, throws); } @@ -124,7 +124,7 @@ internal override void StatementTimeoutTest(StatementWithExceptions statementWit [InlineData(LongRunningStatementTimeoutTestData.LongRunningQuery, "true", "false")] internal async Task DatabricksCanCancelStatementTest(string query, string enableRunAsyncInThriftOp, string enableDirectResults) { - if (TestConfiguration.Protocol == "rest") + if (DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration)) { // SEA: Thrift flags don't apply; run basic cancel test directly await base.CanCancelStatementTest(query); @@ -485,7 +485,7 @@ public async Task CanGetColumnsWithBaseTypeName() [InlineData("all_column_types", "Resources/create_table_all_types.sql", "Resources/result_get_column_extended_all_types.json", false, new[] { "PK_IS_NULLABLE:YES" })] public async Task CanGetColumnsExtended(string tableName, string createTableSqlLocation, string resultLocation, bool useDescTableExtended, string[]? extraPlaceholdsInResult = null) { - Skip.If(TestConfiguration.Protocol == "rest", "SEA CanGetColumnsExtended returns different BUFFER_LENGTH values (PECO-3008)"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA CanGetColumnsExtended returns different BUFFER_LENGTH values (PECO-3008)"); var connectionParams = new Dictionary { ["adbc.databricks.use_desc_table_extended"] = $"{useDescTableExtended}" }; using AdbcConnection connection = NewConnection(TestConfiguration, connectionParams); @@ -1196,7 +1196,7 @@ private void AssertField(Schema schema, int index, string expectedName, IArrowTy [InlineData(true, "main", false)] public void ShouldReturnEmptyPkFkResult_WorksAsExpected(bool enablePKFK, string? catalogName, bool expected) { - Skip.If(TestConfiguration.Protocol == "rest", "SEA: hard cast to DatabricksStatement fails for StatementExecutionStatement"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA: hard cast to DatabricksStatement fails for StatementExecutionStatement"); // Arrange: create test configuration and connection var testConfig = (DatabricksTestConfiguration)TestConfiguration.Clone(); var connectionParams = new Dictionary diff --git a/csharp/test/E2E/StringValueTests.cs b/csharp/test/E2E/StringValueTests.cs index 607fa75f7..b5b82b414 100644 --- a/csharp/test/E2E/StringValueTests.cs +++ b/csharp/test/E2E/StringValueTests.cs @@ -72,7 +72,7 @@ protected override async Task TestVarcharExceptionData(string value, string[] ex public async Task TestVarcharExceptionDataDatabricks(string value, string[] expectedTexts, string? expectedSqlState) { // TODO: PECO-3014 - SEA throws DatabricksException not HiveServer2Exception; Assert.Throws() fails - Skip.If(TestConfiguration.Protocol == "rest", "SEA throws DatabricksException not HiveServer2Exception"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "SEA throws DatabricksException not HiveServer2Exception"); await base.TestVarcharExceptionData(value, expectedTexts, expectedSqlState); } } diff --git a/csharp/test/E2E/Telemetry/AuthTypeTests.cs b/csharp/test/E2E/Telemetry/AuthTypeTests.cs index 172518ec7..1feccad03 100644 --- a/csharp/test/E2E/Telemetry/AuthTypeTests.cs +++ b/csharp/test/E2E/Telemetry/AuthTypeTests.cs @@ -37,7 +37,7 @@ public AuthTypeTests(ITestOutputHelper? outputHelper) : base(outputHelper, new DatabricksTestEnvironment.Factory()) { Skip.IfNot(Utils.CanExecuteTestConfig(TestConfigVariable)); - Skip.If(TestConfiguration.Protocol == "rest", "Telemetry not wired for SEA protocol (PECO-3010)"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "Telemetry not wired for SEA protocol (PECO-3010)"); } /// diff --git a/csharp/test/E2E/Telemetry/ChunkDetailsTelemetryTests.cs b/csharp/test/E2E/Telemetry/ChunkDetailsTelemetryTests.cs index 6b3dcf9d6..9779aad3c 100644 --- a/csharp/test/E2E/Telemetry/ChunkDetailsTelemetryTests.cs +++ b/csharp/test/E2E/Telemetry/ChunkDetailsTelemetryTests.cs @@ -41,7 +41,7 @@ public ChunkDetailsTelemetryTests(ITestOutputHelper? outputHelper) : base(outputHelper, new DatabricksTestEnvironment.Factory()) { Skip.IfNot(Utils.CanExecuteTestConfig(TestConfigVariable)); - Skip.If(TestConfiguration.Protocol == "rest", "CloudFetch telemetry tests are Thrift-only"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "CloudFetch telemetry tests are Thrift-only"); } /// diff --git a/csharp/test/E2E/Telemetry/ChunkMetricsAggregationTests.cs b/csharp/test/E2E/Telemetry/ChunkMetricsAggregationTests.cs index 9d04807f7..6f73a9769 100644 --- a/csharp/test/E2E/Telemetry/ChunkMetricsAggregationTests.cs +++ b/csharp/test/E2E/Telemetry/ChunkMetricsAggregationTests.cs @@ -36,7 +36,7 @@ public ChunkMetricsAggregationTests(ITestOutputHelper? outputHelper) : base(outputHelper, new DatabricksTestEnvironment.Factory()) { Skip.IfNot(Utils.CanExecuteTestConfig(TestConfigVariable)); - Skip.If(TestConfiguration.Protocol == "rest", "CloudFetch metrics tests are Thrift-only"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "CloudFetch metrics tests are Thrift-only"); } /// diff --git a/csharp/test/E2E/Telemetry/ChunkMetricsReaderTests.cs b/csharp/test/E2E/Telemetry/ChunkMetricsReaderTests.cs index dfd65a9c8..a94e8cf19 100644 --- a/csharp/test/E2E/Telemetry/ChunkMetricsReaderTests.cs +++ b/csharp/test/E2E/Telemetry/ChunkMetricsReaderTests.cs @@ -38,7 +38,7 @@ public ChunkMetricsReaderTests(ITestOutputHelper? outputHelper) : base(outputHelper, new DatabricksTestEnvironment.Factory()) { Skip.IfNot(Utils.CanExecuteTestConfig(TestConfigVariable)); - Skip.If(TestConfiguration.Protocol == "rest", "CloudFetch metrics reader tests are Thrift-only"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "CloudFetch metrics reader tests are Thrift-only"); } /// diff --git a/csharp/test/E2E/Telemetry/ClientTelemetryE2ETests.cs b/csharp/test/E2E/Telemetry/ClientTelemetryE2ETests.cs index 90c2b2d2d..5296be320 100644 --- a/csharp/test/E2E/Telemetry/ClientTelemetryE2ETests.cs +++ b/csharp/test/E2E/Telemetry/ClientTelemetryE2ETests.cs @@ -41,7 +41,7 @@ public class ClientTelemetryE2ETests : TestBase diff --git a/csharp/test/E2E/Telemetry/ConnectionParametersTests.cs b/csharp/test/E2E/Telemetry/ConnectionParametersTests.cs index 0383adb03..5d1411863 100644 --- a/csharp/test/E2E/Telemetry/ConnectionParametersTests.cs +++ b/csharp/test/E2E/Telemetry/ConnectionParametersTests.cs @@ -38,7 +38,7 @@ public ConnectionParametersTests(ITestOutputHelper? outputHelper) : base(outputHelper, new DatabricksTestEnvironment.Factory()) { Skip.IfNot(Utils.CanExecuteTestConfig(TestConfigVariable)); - Skip.If(TestConfiguration.Protocol == "rest", "Connection parameters telemetry tests are Thrift-only"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "Connection parameters telemetry tests are Thrift-only"); } /// diff --git a/csharp/test/E2E/Telemetry/InternalCallTests.cs b/csharp/test/E2E/Telemetry/InternalCallTests.cs index ac598417a..f4b54efb0 100644 --- a/csharp/test/E2E/Telemetry/InternalCallTests.cs +++ b/csharp/test/E2E/Telemetry/InternalCallTests.cs @@ -39,7 +39,7 @@ public InternalCallTests(ITestOutputHelper? outputHelper) : base(outputHelper, new DatabricksTestEnvironment.Factory()) { Skip.IfNot(Utils.CanExecuteTestConfig(TestConfigVariable)); - Skip.If(TestConfiguration.Protocol == "rest", "Telemetry not wired for SEA protocol (PECO-3010)"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "Telemetry not wired for SEA protocol (PECO-3010)"); } /// diff --git a/csharp/test/E2E/Telemetry/MetadataOperationTests.cs b/csharp/test/E2E/Telemetry/MetadataOperationTests.cs index fe6b3349e..02c6cb894 100644 --- a/csharp/test/E2E/Telemetry/MetadataOperationTests.cs +++ b/csharp/test/E2E/Telemetry/MetadataOperationTests.cs @@ -38,7 +38,7 @@ public MetadataOperationTests(ITestOutputHelper? outputHelper) : base(outputHelper, new DatabricksTestEnvironment.Factory()) { Skip.IfNot(Utils.CanExecuteTestConfig(TestConfigVariable)); - Skip.If(TestConfiguration.Protocol == "rest", "Telemetry not wired for SEA protocol (PECO-3010)"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "Telemetry not wired for SEA protocol (PECO-3010)"); } [SkippableFact] diff --git a/csharp/test/E2E/Telemetry/RetryCountTests.cs b/csharp/test/E2E/Telemetry/RetryCountTests.cs index 8feccb621..7553bff2b 100644 --- a/csharp/test/E2E/Telemetry/RetryCountTests.cs +++ b/csharp/test/E2E/Telemetry/RetryCountTests.cs @@ -40,7 +40,7 @@ public class RetryCountTests : TestBase diff --git a/csharp/test/E2E/Telemetry/StatementMetadataTelemetryTests.cs b/csharp/test/E2E/Telemetry/StatementMetadataTelemetryTests.cs index 48032c27f..a74ca989b 100644 --- a/csharp/test/E2E/Telemetry/StatementMetadataTelemetryTests.cs +++ b/csharp/test/E2E/Telemetry/StatementMetadataTelemetryTests.cs @@ -48,7 +48,7 @@ public StatementMetadataTelemetryTests(ITestOutputHelper? outputHelper) : base(outputHelper, new DatabricksTestEnvironment.Factory()) { Skip.IfNot(Utils.CanExecuteTestConfig(TestConfigVariable)); - Skip.If(TestConfiguration.Protocol == "rest", "Telemetry not wired for SEA protocol (PECO-3010)"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "Telemetry not wired for SEA protocol (PECO-3010)"); } [SkippableFact] diff --git a/csharp/test/E2E/Telemetry/SystemConfigurationTests.cs b/csharp/test/E2E/Telemetry/SystemConfigurationTests.cs index 1908b6d38..d092e5446 100644 --- a/csharp/test/E2E/Telemetry/SystemConfigurationTests.cs +++ b/csharp/test/E2E/Telemetry/SystemConfigurationTests.cs @@ -38,7 +38,7 @@ public SystemConfigurationTests(ITestOutputHelper? outputHelper) : base(outputHelper, new DatabricksTestEnvironment.Factory()) { Skip.IfNot(Utils.CanExecuteTestConfig(TestConfigVariable)); - Skip.If(TestConfiguration.Protocol == "rest", "Telemetry not wired for SEA protocol (PECO-3010)"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "Telemetry not wired for SEA protocol (PECO-3010)"); } /// diff --git a/csharp/test/E2E/Telemetry/TelemetryBaselineTests.cs b/csharp/test/E2E/Telemetry/TelemetryBaselineTests.cs index d8ad2e68b..c1e903c94 100644 --- a/csharp/test/E2E/Telemetry/TelemetryBaselineTests.cs +++ b/csharp/test/E2E/Telemetry/TelemetryBaselineTests.cs @@ -40,7 +40,7 @@ public TelemetryBaselineTests(ITestOutputHelper? outputHelper) : base(outputHelper, new DatabricksTestEnvironment.Factory()) { Skip.IfNot(Utils.CanExecuteTestConfig(TestConfigVariable)); - Skip.If(TestConfiguration.Protocol == "rest", "Telemetry not wired for SEA protocol (PECO-3010)"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "Telemetry not wired for SEA protocol (PECO-3010)"); } /// diff --git a/csharp/test/E2E/TelemetryTests.cs b/csharp/test/E2E/TelemetryTests.cs index 20ce0f89e..586ca5b3e 100644 --- a/csharp/test/E2E/TelemetryTests.cs +++ b/csharp/test/E2E/TelemetryTests.cs @@ -33,7 +33,7 @@ public TelemetryTests(ITestOutputHelper outputHelper) : base(outputHelper, new DatabricksTestEnvironment.Factory()) { // TODO: PECO-3010 - telemetry not wired for SEA protocol; file trace exporter produces no output - Skip.If(TestConfiguration.Protocol == "rest", "Telemetry file tracing is Thrift-only"); + Skip.If(DatabricksTestEnvironment.IsResolvedProtocolRest(TestConfiguration), "Telemetry file tracing is Thrift-only"); } } }