Skip to content

Commit fde0c3f

Browse files
fix(csharp): silently drop unrecognized SetOption flags for compatibility (#348)
## Summary - When clients like Power BI set options not yet implemented in this driver (e.g. `adbc.databricks.query_tags` from an older Mashup version), the driver was throwing `AdbcException.NotImplemented` causing unexpected failures - `DatabricksStatement.SetOption` (Thrift path): calls `base.SetOption(key, value)` and catches `AdbcStatusCode.NotImplemented`, silently dropping unknown options while preserving correct handling of any base-class options (e.g. `PollTimeMilliseconds`, `QueryTimeoutSeconds`) - `StatementExecutionStatement.SetOption` (SEA path): same pattern — calls `base.SetOption(key, value)` and catches `NotImplemented` to silently drop unknown options - Both paths emit an `Activity` event (`statement.set_option.unrecognized`) with the dropped key for observability ## Test plan - [x] Added `StatementSetOptionTests.SetOption_UnrecognizedKey_DoesNotThrow` (SEA path, `StatementExecutionStatement`) - [x] Added `DatabricksStatementTests.SetOption_UnrecognizedKey_DoesNotThrow` (Thrift path, `DatabricksStatement`) - [x] All 633 unit tests pass - [x] Build succeeds for all target frameworks (`netstandard2.0`, `net472`, `net8.0`) Closes PECO-2952 This pull request was AI-assisted by Isaac. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3c1dbe0 commit fde0c3f

File tree

5 files changed

+115
-3
lines changed

5 files changed

+115
-3
lines changed

csharp/src/DatabricksStatement.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,17 @@ public override void SetOption(string key, string value)
382382
}
383383
break;
384384
default:
385-
base.SetOption(key, value);
385+
try
386+
{
387+
base.SetOption(key, value);
388+
}
389+
catch (AdbcException ex) when (ex.Status == AdbcStatusCode.NotImplemented)
390+
{
391+
// Silently drop unrecognized options for compatibility with clients
392+
// that may set options not yet supported by this driver.
393+
Activity.Current?.AddEvent(new ActivityEvent("statement.set_option.unrecognized",
394+
tags: new ActivityTagsCollection { { "key", key } }));
395+
}
386396
break;
387397
}
388398
}

csharp/src/StatementExecution/StatementExecutionStatement.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,17 @@ public override void SetOption(string key, string value)
199199
break;
200200

201201
default:
202-
base.SetOption(key, value);
202+
try
203+
{
204+
base.SetOption(key, value);
205+
}
206+
catch (AdbcException ex) when (ex.Status == AdbcStatusCode.NotImplemented)
207+
{
208+
// Silently drop unrecognized options for compatibility with clients
209+
// that may set options not yet supported by this driver.
210+
Activity.Current?.AddEvent(new ActivityEvent("statement.set_option.unrecognized",
211+
tags: new ActivityTagsCollection { { "key", key } }));
212+
}
203213
break;
204214
}
205215
}

csharp/test/Unit/DatabricksStatementUnitTests.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,5 +126,15 @@ public void CreateStatement_ConfOverlayInitiallyNull()
126126
var confOverlay = GetConfOverlay(statement);
127127
Assert.Null(confOverlay);
128128
}
129+
130+
/// <summary>
131+
/// Tests that unrecognized options are silently dropped instead of throwing (PECO-2952).
132+
/// </summary>
133+
[Fact]
134+
public void SetOption_UnrecognizedKey_DoesNotThrow()
135+
{
136+
using var statement = CreateStatement();
137+
statement.SetOption("adbc.databricks.unknown_future_option", "some_value");
138+
}
129139
}
130140
}

csharp/test/Unit/FeatureFlagCacheTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ public async Task FeatureFlagContext_ConcurrentFlagAccess_ThreadSafe()
418418
public async Task FeatureFlagContext_BackgroundRefreshError_SetsActivityStatusToError()
419419
{
420420
// Arrange
421-
var backgroundActivities = new List<(string Name, ActivityStatusCode Status, List<ActivityEvent> Events)>();
421+
var backgroundActivities = new System.Collections.Concurrent.ConcurrentBag<(string Name, ActivityStatusCode Status, List<ActivityEvent> Events)>();
422422

423423
var listener = new ActivityListener
424424
{
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Copyright (c) 2025 ADBC Drivers Contributors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
using System.Collections.Generic;
18+
using System.Net;
19+
using System.Net.Http;
20+
using System.Text.Json;
21+
using System.Threading;
22+
using System.Threading.Tasks;
23+
using AdbcDrivers.Databricks.StatementExecution;
24+
using AdbcDrivers.HiveServer2;
25+
using AdbcDrivers.HiveServer2.Spark;
26+
using Microsoft.IO;
27+
using Moq;
28+
using Moq.Protected;
29+
using Xunit;
30+
31+
namespace AdbcDrivers.Databricks.Tests.Unit.StatementExecution
32+
{
33+
/// <summary>
34+
/// Unit tests verifying that unrecognized options are silently dropped by
35+
/// StatementExecutionStatement.SetOption instead of throwing exceptions (PECO-2952).
36+
/// </summary>
37+
public class StatementSetOptionTests
38+
{
39+
[Fact]
40+
public void SetOption_UnrecognizedKey_DoesNotThrow()
41+
{
42+
var properties = new Dictionary<string, string>
43+
{
44+
{ SparkParameters.HostName, "test.databricks.com" },
45+
{ DatabricksParameters.WarehouseId, "wh-1" },
46+
{ SparkParameters.AccessToken, "token" },
47+
};
48+
49+
var handlerMock = new Mock<HttpMessageHandler>();
50+
handlerMock.Protected()
51+
.Setup<Task<HttpResponseMessage>>("SendAsync",
52+
ItExpr.IsAny<HttpRequestMessage>(),
53+
ItExpr.IsAny<CancellationToken>())
54+
.ReturnsAsync(new HttpResponseMessage(HttpStatusCode.OK)
55+
{
56+
Content = new StringContent(
57+
JsonSerializer.Serialize(new { session_id = "s1" }))
58+
});
59+
60+
using var httpClient = new HttpClient(handlerMock.Object);
61+
using var connection = new StatementExecutionConnection(properties, httpClient);
62+
using var statement = new StatementExecutionStatement(
63+
client: Mock.Of<IStatementExecutionClient>(),
64+
sessionId: "session-1",
65+
warehouseId: "wh-1",
66+
catalog: null,
67+
schema: null,
68+
resultDisposition: "INLINE_OR_EXTERNAL_LINKS",
69+
resultFormat: "ARROW_STREAM",
70+
resultCompression: null,
71+
waitTimeoutSeconds: 0,
72+
pollingIntervalMs: 50,
73+
properties: properties,
74+
recyclableMemoryStreamManager: new RecyclableMemoryStreamManager(),
75+
lz4BufferPool: System.Buffers.ArrayPool<byte>.Shared,
76+
httpClient: httpClient,
77+
connection: connection);
78+
79+
statement.SetOption("adbc.databricks.unknown_future_option", "some_value");
80+
}
81+
}
82+
}

0 commit comments

Comments
 (0)