Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions csharp/src/StatementExecution/StatementExecutionConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,11 @@ private void ValidateProperties()
// PECO-3064: adbc.apache.statement.polltime_ms is the single key for SEA polling cadence
// (consolidated with the Thrift path). SEA defaults to 1000 ms — HTTP/JSON polls are
// heavier than Thrift's 100 ms default — but both protocols share the same property.
_pollingIntervalMs = PropertyHelper.GetPositiveIntPropertyWithValidation(
properties, ApacheParameters.PollTimeMilliseconds, defaultValue: 1000);
// Validated via StatementOptionValidator so an invalid poll time throws
// ArgumentOutOfRangeException (matching Thrift), allowing 0 (no delay) and up.
_pollingIntervalMs = properties.TryGetValue(ApacheParameters.PollTimeMilliseconds, out string? pollTimeValue)
? StatementOptionValidator.ValidatePollTime(ApacheParameters.PollTimeMilliseconds, pollTimeValue)
: 1000;

// Tracing propagation configuration. Base class (TracingConnection) already handles ActivityTrace init.
_tracePropagationEnabled = PropertyHelper.GetBooleanPropertyWithValidation(properties, DatabricksParameters.TracePropagationEnabled, true);
Expand Down
25 changes: 21 additions & 4 deletions csharp/src/StatementExecution/StatementExecutionStatement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,17 @@ public StatementExecutionStatement(
_waitTimeoutSeconds = waitTimeoutSeconds;
_pollingIntervalMs = pollingIntervalMs;
_properties = properties ?? throw new ArgumentNullException(nameof(properties));
_queryTimeoutSeconds = PropertyHelper.GetIntPropertyWithValidation(
properties, ApacheParameters.QueryTimeoutSeconds, 0);

// Validate config-supplied statement options so bad config throws ArgumentOutOfRangeException
// at CreateStatement time, matching the Thrift path. BatchSize is read-only on SEA — validated
// for parity but otherwise unused at the statement level.
_queryTimeoutSeconds = properties.TryGetValue(ApacheParameters.QueryTimeoutSeconds, out string? queryTimeoutValue)
? StatementOptionValidator.ValidateQueryTimeout(ApacheParameters.QueryTimeoutSeconds, queryTimeoutValue)
: 0;
if (properties.TryGetValue(ApacheParameters.BatchSize, out string? batchSizeValue))
{
StatementOptionValidator.ValidateBatchSize(ApacheParameters.BatchSize, batchSizeValue);
}
_recyclableMemoryStreamManager = recyclableMemoryStreamManager ?? throw new ArgumentNullException(nameof(recyclableMemoryStreamManager));
_lz4BufferPool = lz4BufferPool ?? throw new ArgumentNullException(nameof(lz4BufferPool));
_httpClient = httpClient ?? throw new ArgumentNullException(nameof(httpClient));
Expand Down Expand Up @@ -237,12 +246,20 @@ public override void SetOption(string key, string value)
_escapePatternWildcards = bool.TryParse(value, out bool escape) && escape;
break;

// These options are readonly in SEA (set at connection level).
// Accept but ignore them to avoid NotImplemented exceptions for compatibility.
// Validate these options to match Thrift path behavior (throw ArgumentOutOfRangeException
// for invalid values), even though they are read-only in SEA (set at connection level).
// The same validators run at construction (see constructor / connection), so SetOption
// and bad-config behave identically to Thrift.
case ApacheParameters.PollTimeMilliseconds:
StatementOptionValidator.ValidatePollTime(key, value);
break;
case ApacheParameters.BatchSize:
StatementOptionValidator.ValidateBatchSize(key, value);
break;
case ApacheParameters.BatchSizeStopCondition:
break;
case ApacheParameters.QueryTimeoutSeconds:
StatementOptionValidator.ValidateQueryTimeout(key, value);
break;

case DatabricksParameters.QueryTags:
Expand Down
64 changes: 64 additions & 0 deletions csharp/src/StatementExecution/StatementOptionValidator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* 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;

namespace AdbcDrivers.Databricks.StatementExecution
{
/// <summary>
/// Validates the numeric statement options shared with the Thrift path
/// (poll time, batch size, query timeout). Each method throws
/// <see cref="ArgumentOutOfRangeException"/> for any invalid value — non-numeric,
/// out of range, or overflowing — with the exact message text the Thrift driver uses
/// (HiveServer2Statement / ApacheUtility), so SEA and Thrift behave identically.
///
/// On SEA these options are read-only (set at connection level); the validators exist
/// purely for API/behavior parity — they validate but the value is otherwise unused at
/// the statement level.
/// </summary>
internal static class StatementOptionValidator
{
/// <summary>Poll-time in milliseconds: a numeric value greater than or equal to 0.</summary>
public static int ValidatePollTime(string key, string value)
{
if (string.IsNullOrEmpty(value) || !int.TryParse(value, out int pollTimeMs) || pollTimeMs < 0)
{
throw new ArgumentOutOfRangeException(key, value, $"The value '{value}' for option '{key}' is invalid. Must be a numeric value greater than or equal to 0.");
}
return pollTimeMs;
}

/// <summary>Batch size: a numeric (long) value greater than zero.</summary>
public static long ValidateBatchSize(string key, string value)
{
if (string.IsNullOrEmpty(value) || !long.TryParse(value, out long batchSize) || batchSize <= 0)
{
throw new ArgumentOutOfRangeException(key, value, $"The value '{value}' for option '{key}' is invalid. Must be a numeric value greater than zero.");
}
return batchSize;
}

/// <summary>Query timeout in seconds: a numeric value of 0 (infinite) or greater.</summary>
public static int ValidateQueryTimeout(string key, string value)
{
if (string.IsNullOrEmpty(value) || !int.TryParse(value, out int queryTimeoutSecs) || queryTimeoutSecs < 0)
{
throw new ArgumentOutOfRangeException(key, value, $"The value '{value}' for option '{key}' is invalid. Must be a numeric value of 0 (infinite) or greater.");
}
return queryTimeoutSecs;
}
}
}
24 changes: 3 additions & 21 deletions csharp/test/E2E/StatementTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,34 +37,16 @@

namespace AdbcDrivers.Databricks.Tests
{
// TODO: PECO-3011 - CanSetOptionBatchSize/PollTime/QueryTimeout inherited from base class not validated in SEA's StatementExecutionStatement
public class StatementTests : StatementTests<DatabricksTestConfiguration, DatabricksTestEnvironment>
{
public StatementTests(ITestOutputHelper? outputHelper)
: base(outputHelper, new DatabricksTestEnvironment.Factory())
{
}

// 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");
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");
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");
base.ValidateCanSetOptionBatchSize(value, throws);
}
// PECO-3011: SEA now validates PollTime/QueryTimeout/BatchSize at both construction and
// SetOption (StatementOptionValidator), matching the Thrift path — so the inherited
// CanSetOption* theories run unmodified on both protocols (no SEA skip override needed).

[SkippableTheory]
[InlineData(true, "CloudFetch enabled")]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* 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;
using AdbcDrivers.Databricks.StatementExecution;
using Xunit;

namespace AdbcDrivers.Databricks.Tests.Unit.StatementExecution
{
/// <summary>
/// Pins the SEA statement-option validation to match the Thrift path (PECO-3011): every invalid
/// value throws <see cref="ArgumentOutOfRangeException"/>. The InlineData mirrors the inherited
/// CanSetOption{PollTime,BatchSize,QueryTimeout} theories so the local check tracks the E2E ones.
/// </summary>
public class StatementOptionValidatorTests
{
// --- PollTime: numeric >= 0 ---
[Theory]
[InlineData("-1")]
[InlineData("zero")]
[InlineData("-2147483648")]
[InlineData("2147483648")] // overflows int
[InlineData("")]
public void ValidatePollTime_Invalid_Throws(string value) =>
Assert.Throws<ArgumentOutOfRangeException>(() => StatementOptionValidator.ValidatePollTime("k", value));

[Theory]
[InlineData("0", 0)]
[InlineData("1", 1)]
[InlineData("2147483647", 2147483647)]
public void ValidatePollTime_Valid_ReturnsValue(string value, int expected) =>
Assert.Equal(expected, StatementOptionValidator.ValidatePollTime("k", value));

// --- BatchSize: numeric (long) > 0 ---
[Theory]
[InlineData("-1")]
[InlineData("one")]
[InlineData("-2147483648")]
[InlineData("9223372036854775808")] // overflows long
[InlineData("0")]
[InlineData("")]
public void ValidateBatchSize_Invalid_Throws(string value) =>
Assert.Throws<ArgumentOutOfRangeException>(() => StatementOptionValidator.ValidateBatchSize("k", value));

[Theory]
[InlineData("1", 1L)]
[InlineData("2147483648", 2147483648L)]
[InlineData("9223372036854775807", 9223372036854775807L)]
public void ValidateBatchSize_Valid_ReturnsValue(string value, long expected) =>
Assert.Equal(expected, StatementOptionValidator.ValidateBatchSize("k", value));

// --- QueryTimeout: numeric >= 0 (0 = infinite) ---
[Theory]
[InlineData("-1")]
[InlineData("zero")]
[InlineData("-2147483648")]
[InlineData("2147483648")]
[InlineData("")]
public void ValidateQueryTimeout_Invalid_Throws(string value) =>
Assert.Throws<ArgumentOutOfRangeException>(() => StatementOptionValidator.ValidateQueryTimeout("k", value));

[Theory]
[InlineData("0", 0)]
[InlineData("60", 60)]
public void ValidateQueryTimeout_Valid_ReturnsValue(string value, int expected) =>
Assert.Equal(expected, StatementOptionValidator.ValidateQueryTimeout("k", value));

[Fact]
public void Messages_MatchThriftWording()
{
var poll = Assert.Throws<ArgumentOutOfRangeException>(() => StatementOptionValidator.ValidatePollTime("k", "-1"));
Assert.Contains("Must be a numeric value greater than or equal to 0.", poll.Message);

var batch = Assert.Throws<ArgumentOutOfRangeException>(() => StatementOptionValidator.ValidateBatchSize("k", "0"));
Assert.Contains("Must be a numeric value greater than zero.", batch.Message);

var timeout = Assert.Throws<ArgumentOutOfRangeException>(() => StatementOptionValidator.ValidateQueryTimeout("k", "-1"));
Assert.Contains("Must be a numeric value of 0 (infinite) or greater.", timeout.Message);
}
}
}
Loading