From 7a63ef6b661d78e997b4091ed0499b704c442ab6 Mon Sep 17 00:00:00 2001 From: David Coe Date: Wed, 4 Dec 2024 17:31:41 -0500 Subject: [PATCH 1/5] add timeout parsing from connection string --- csharp/src/Client/AdbcCommand.cs | 9 +- csharp/src/Client/AdbcConnection.cs | 54 ++++++++- csharp/src/Client/ConnectionStringParser.cs | 84 ++++++++++++++ csharp/src/Client/readme.md | 9 ++ .../Client/ClientTests.cs | 104 ++++++++++++++++++ 5 files changed, 256 insertions(+), 4 deletions(-) create mode 100644 csharp/src/Client/ConnectionStringParser.cs diff --git a/csharp/src/Client/AdbcCommand.cs b/csharp/src/Client/AdbcCommand.cs index c3695feaf4..a317ca19cc 100644 --- a/csharp/src/Client/AdbcCommand.cs +++ b/csharp/src/Client/AdbcCommand.cs @@ -53,6 +53,7 @@ public AdbcCommand(AdbcConnection adbcConnection) : base() this.DbConnection = adbcConnection; this.DecimalBehavior = adbcConnection.DecimalBehavior; + this.StructBehavior = adbcConnection.StructBehavior; this._adbcStatement = adbcConnection.CreateStatement(); } @@ -74,6 +75,7 @@ public AdbcCommand(string query, AdbcConnection adbcConnection) : base() this.DbConnection = adbcConnection; this.DecimalBehavior = adbcConnection.DecimalBehavior; + this.StructBehavior = adbcConnection.StructBehavior; } // For testing @@ -83,6 +85,12 @@ internal AdbcCommand(AdbcStatement adbcStatement, AdbcConnection adbcConnection) this.DbConnection = adbcConnection; this.DecimalBehavior = adbcConnection.DecimalBehavior; this.StructBehavior = adbcConnection.StructBehavior; + + if (adbcConnection.CommandTimeoutValue != null) + { + this.AdbcCommandTimeoutProperty = adbcConnection.CommandTimeoutValue.DriverPropertyName; + this.CommandTimeout = adbcConnection.CommandTimeoutValue.Value; + } } /// @@ -119,7 +127,6 @@ public override CommandType CommandType } } - /// /// Gets or sets the name of the command timeout property for the underlying ADBC driver. /// diff --git a/csharp/src/Client/AdbcConnection.cs b/csharp/src/Client/AdbcConnection.cs index 2b35c92d93..673ebf80c0 100644 --- a/csharp/src/Client/AdbcConnection.cs +++ b/csharp/src/Client/AdbcConnection.cs @@ -34,6 +34,7 @@ public sealed class AdbcConnection : DbConnection { private AdbcDatabase? adbcDatabase; private Adbc.AdbcConnection? adbcConnectionInternal; + private TimeoutValue? connectionTimeoutValue; private readonly Dictionary adbcConnectionParameters; private readonly Dictionary adbcConnectionOptions; @@ -105,7 +106,10 @@ internal AdbcConnection(AdbcDriver driver, AdbcDatabase database, Adbc.AdbcConne /// Creates a new . /// /// - public new AdbcCommand CreateCommand() => (AdbcCommand)CreateDbCommand(); + public new AdbcCommand CreateCommand() + { + return (AdbcCommand)CreateDbCommand(); + } /// /// Gets or sets the associated with this @@ -122,6 +126,8 @@ internal AdbcStatement CreateStatement() return this.adbcConnectionInternal!.CreateStatement(); } + internal TimeoutValue? CommandTimeoutValue { get; private set; } + #if NET5_0_OR_GREATER [AllowNull] #endif @@ -137,11 +143,35 @@ internal AdbcStatement CreateStatement() /// public StructBehavior StructBehavior { get; set; } = StructBehavior.JsonString; + public override int ConnectionTimeout + { + get + { + if (connectionTimeoutValue != null) + return connectionTimeoutValue.Value; + else + return base.ConnectionTimeout; + } + } + protected override DbCommand CreateDbCommand() { EnsureConnectionOpen(); - return new AdbcCommand(this); + return GetAdbcCommand(); + } + + private AdbcCommand GetAdbcCommand() + { + AdbcCommand cmd = new AdbcCommand(this); + + if (CommandTimeoutValue != null) + { + cmd.AdbcCommandTimeoutProperty = CommandTimeoutValue.DriverPropertyName!; + cmd.CommandTimeout = CommandTimeoutValue.Value; + } + + return cmd; } /// @@ -237,7 +267,25 @@ private void SetConnectionProperties(string value) object? builderValue = builder[key]; if (builderValue != null) { - this.adbcConnectionParameters.Add(key, Convert.ToString(builderValue)!); + string paramValue = Convert.ToString(builderValue)!; + + this.adbcConnectionParameters.Add(key, paramValue); + + switch (key) + { + case ConnectionStringKeywords.DecimalBehavior: + this.DecimalBehavior = (DecimalBehavior)Enum.Parse(typeof(DecimalBehavior), paramValue); + break; + case ConnectionStringKeywords.StructBehavior: + this.StructBehavior = (StructBehavior)Enum.Parse(typeof(StructBehavior), paramValue); + break; + case ConnectionStringKeywords.CommandTimeout: + CommandTimeoutValue = ConnectionStringParser.ParseTimeoutValue(paramValue); + break; + case ConnectionStringKeywords.ConnectionTimeout: + this.connectionTimeoutValue = ConnectionStringParser.ParseTimeoutValue(paramValue); + break; + } } } } diff --git a/csharp/src/Client/ConnectionStringParser.cs b/csharp/src/Client/ConnectionStringParser.cs new file mode 100644 index 0000000000..9b47246ac7 --- /dev/null +++ b/csharp/src/Client/ConnectionStringParser.cs @@ -0,0 +1,84 @@ +/* +* Licensed to the Apache Software Foundation (ASF) under one or more +* contributor license agreements. See the NOTICE file distributed with +* this work for additional information regarding copyright ownership. +* The ASF licenses this file to You 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 System.Collections.Generic; +using System.Net.Http.Headers; +using System.Text; +using System.Text.RegularExpressions; + +namespace Apache.Arrow.Adbc.Client +{ + internal class ConnectionStringKeywords + { + public const string ConnectionTimeout = "adbcconnectiontimeout"; + public const string CommandTimeout = "adbccommandtimeout"; + public const string StructBehavior = "structbehavior"; + public const string DecimalBehavior = "decimalbehavior"; + } + + internal class ConnectionStringParser + { + public static TimeoutValue ParseTimeoutValue(string value) + { + string pattern = @"\(([^,]+),\s*([^,]+),\s*([^,]+)\)"; + + // Match the regex + Match match = Regex.Match(value, pattern); + + if (match.Success) + { + string driverPropertyName = match.Groups[1].Value.Trim(); + string timeoutAsString = match.Groups[2].Value.Trim(); + string units = match.Groups[3].Value.Trim(); + + if (units != "s" && units != "ms") + { + throw new InvalidOperationException("invalid units"); + } + + TimeoutValue timeoutValue = new TimeoutValue + { + DriverPropertyName = driverPropertyName, + Value = int.Parse(timeoutAsString), + Units = units + }; + + return timeoutValue; + } + else + { + throw new InvalidOperationException("cannot parse the timeout value"); + } + } + } + + internal class TimeoutValue + { + public string DriverPropertyName { get; set; } = string.Empty; + + public int Value { get; set; } + + // seconds=s + // milliseconds=ms + /// + /// While these can be helpful, the DbConnection and DbCommand + /// objects limit the use of these. + /// + public string Units { get; set; } = string.Empty; + } +} diff --git a/csharp/src/Client/readme.md b/csharp/src/Client/readme.md index 2c4a51aca3..59f647a37e 100644 --- a/csharp/src/Client/readme.md +++ b/csharp/src/Client/readme.md @@ -73,3 +73,12 @@ if using the default user name and password authentication, but look like when using JWT authentication with an unencrypted key file. Other ADBC drivers will have different connection parameters, so be sure to check the documentation for each driver. + +### Connection Keywords +Because the ADO.NET client is designed to work with multiple drivers, callers will need to specify the driver properties that are set for particular values. This can be done either as properties on the objects directly, or can be parsed from the connection string. +These properties are: + +- __AdbcConnectionTimeout__ - This specifies the connection timeout value. The value needs to be in the form (driver.property.name, integer, unit) where the unit is one of `s` or `ms`, For example, `AdbcConnectionTimeout=(adbc.snowflake.sql.client_option.client_timeout,30,s)` would set the connection timeout to 30 seconds. +- __AdbcCommandTimeout__ - This specifies the command timeout value. This follows the same pattern as `AdbcConnectionTimeout` and sets the `AdbcCommandTimeoutProperty` and `CommandTimeout` values on the `AdbcCommand` object. +- __StructBehavior__ - This specifies the StructBehavior when working with Arrow Struct arrays. The valid values are `JsonString` (the default) or `Strict` (treat the struct as a native type). +- __DecimalBehavior__ - This specifies the DecimalBehavior when parsing decimal values from Arrow libraries. The valid values are `UseSqlDecimal` or `OverflowDecimalAsString` where values like Decimal256 are treated as strings. diff --git a/csharp/test/Apache.Arrow.Adbc.Tests/Client/ClientTests.cs b/csharp/test/Apache.Arrow.Adbc.Tests/Client/ClientTests.cs index e20b270d0a..0eefb811ed 100644 --- a/csharp/test/Apache.Arrow.Adbc.Tests/Client/ClientTests.cs +++ b/csharp/test/Apache.Arrow.Adbc.Tests/Client/ClientTests.cs @@ -17,6 +17,7 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using System.Data.SqlTypes; using System.Linq; using System.Threading; @@ -26,6 +27,7 @@ using Apache.Arrow.Types; using Moq; using Xunit; +using AdbcClient = Apache.Arrow.Adbc.Client; namespace Apache.Arrow.Adbc.Tests.Client { @@ -166,6 +168,108 @@ private AdbcDataReader GetMoqDataReaderForIntegers() AdbcDataReader reader = cmd.ExecuteReader(); return reader; } + + [Theory] + [InlineData("(adbc.driver.value, 1, s)", "adbc.driver.value", 1, "s", true)] + [InlineData("(somevalue,10, ms)", "somevalue", 10, "ms", true)] + [InlineData("(somevalue,10, s)", "somevalue", 10, "s", true)] + [InlineData("somevalue,10, s)", null, null, null, false)] + [InlineData("(somevalue,10, s", null, null, null, false)] + [InlineData("(some.value_goes.here,99,Q)", null, null, null, false)] + [InlineData("some.value_goes.here,99,Q", null, null, null, false)] + public void TestTimeoutParsing(string value, string? driverPropertyName, int? timeout, string? unit, bool success) + { + if (!success) + { + Assert.Throws(() => ConnectionStringParser.ParseTimeoutValue(value)); + } + else + { + Assert.True(driverPropertyName != null); + Assert.True(timeout != null); + Assert.True(unit != null); + + TimeoutValue timeoutValue = ConnectionStringParser.ParseTimeoutValue(value); + + Assert.Equal(driverPropertyName, timeoutValue.DriverPropertyName); + Assert.Equal(timeout, timeoutValue.Value); + Assert.Equal(unit, timeoutValue.Units); + } + } + + [Theory] + [ClassData(typeof(ConnectionParsingTestData))] + internal void TestConnectionStringParsing(ConnectionStringExample connectionStringExample) + { + AdbcClient.AdbcConnection cn = new AdbcClient.AdbcConnection(connectionStringExample.ConnectionString); + + Mock mockStatement = new Mock(); + AdbcCommand cmd = new AdbcCommand(mockStatement.Object, cn); + + Assert.True(cn.StructBehavior == connectionStringExample.ExpectedStructBehavior); + Assert.True(cn.DecimalBehavior == connectionStringExample.ExpectedDecimalBehavior); + Assert.True(cn.ConnectionTimeout == connectionStringExample.ConnectionTimeout); + + if (!string.IsNullOrEmpty(connectionStringExample.CommandTimeoutProperty)) + { + Assert.True(cmd.AdbcCommandTimeoutProperty == connectionStringExample.CommandTimeoutProperty); + Assert.True(cmd.CommandTimeout == connectionStringExample.CommandTimeout); + } + else + { + Assert.Throws(() => cmd.AdbcCommandTimeoutProperty); + } + } + } + + internal class ConnectionStringExample + { + public ConnectionStringExample( + string connectionString, + DecimalBehavior decimalBehavior, + StructBehavior structBehavior, + string connectionTimeoutPropertyName, + int connectionTimeout, + string commandTimeoutPropertyName, + int commandTimeout) + { + ConnectionString = connectionString; + ExpectedDecimalBehavior = decimalBehavior; + ExpectedStructBehavior = structBehavior; + ConnectionTimeoutProperty = connectionTimeoutPropertyName; + ConnectionTimeout = connectionTimeout; + CommandTimeoutProperty = commandTimeoutPropertyName; + CommandTimeout = commandTimeout; + } + + public string ConnectionString { get; } + + public string ConnectionTimeoutProperty { get; } + + public int ConnectionTimeout { get; } + + public DecimalBehavior ExpectedDecimalBehavior { get; } + + public StructBehavior ExpectedStructBehavior { get; } + + public string CommandTimeoutProperty { get; } + + public int CommandTimeout { get; } + } + + /// + /// Collection of for testing statement timeouts."/> + /// + internal class ConnectionParsingTestData : TheoryData + { + public ConnectionParsingTestData() + { + int defaultDbConnectionTimeout = 15; + + Add(new("StructBehavior=JsonString", default, StructBehavior.JsonString, "", defaultDbConnectionTimeout, "", 30)); + Add(new("StructBehavior=JsonString;AdbcCommandTimeout=(adbc.apache.statement.query_timeout_s,45,s)", default, StructBehavior.JsonString, "", defaultDbConnectionTimeout, "adbc.apache.statement.query_timeout_s", 45)); + Add(new("StructBehavior=JsonString;DecimalBehavior=OverflowDecimalAsString;AdbcConnectionTimeout=(adbc.spark.connect_timeout_ms,90,s);AdbcCommandTimeout=(adbc.apache.statement.query_timeout_s,45,s)", DecimalBehavior.OverflowDecimalAsString, StructBehavior.JsonString, "adbc.spark.connect_timeout_ms", 90, "adbc.apache.statement.query_timeout_s", 45)); + } } class MockArrayStream : IArrowArrayStream From 56ca6291d65f3d18676694940bc96400b0140773 Mon Sep 17 00:00:00 2001 From: David Coe Date: Wed, 4 Dec 2024 20:55:32 -0500 Subject: [PATCH 2/5] clean up a few items --- csharp/src/Client/AdbcConnection.cs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/csharp/src/Client/AdbcConnection.cs b/csharp/src/Client/AdbcConnection.cs index 673ebf80c0..331e4f6636 100644 --- a/csharp/src/Client/AdbcConnection.cs +++ b/csharp/src/Client/AdbcConnection.cs @@ -106,10 +106,7 @@ internal AdbcConnection(AdbcDriver driver, AdbcDatabase database, Adbc.AdbcConne /// Creates a new . /// /// - public new AdbcCommand CreateCommand() - { - return (AdbcCommand)CreateDbCommand(); - } + public new AdbcCommand CreateCommand() => (AdbcCommand)CreateDbCommand(); /// /// Gets or sets the associated with this @@ -158,11 +155,6 @@ protected override DbCommand CreateDbCommand() { EnsureConnectionOpen(); - return GetAdbcCommand(); - } - - private AdbcCommand GetAdbcCommand() - { AdbcCommand cmd = new AdbcCommand(this); if (CommandTimeoutValue != null) From e1d70c3469da6c099aa4aa8a57c0d81a497bf5f7 Mon Sep 17 00:00:00 2001 From: David Coe Date: Thu, 5 Dec 2024 17:01:04 -0500 Subject: [PATCH 3/5] PR feedback --- csharp/src/Client/AdbcConnection.cs | 6 ++++-- csharp/src/Client/ConnectionStringParser.cs | 5 +---- .../Apache.Arrow.Adbc.Tests/Client/ClientTests.cs | 11 ++++++++++- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/csharp/src/Client/AdbcConnection.cs b/csharp/src/Client/AdbcConnection.cs index 331e4f6636..c0a1af0528 100644 --- a/csharp/src/Client/AdbcConnection.cs +++ b/csharp/src/Client/AdbcConnection.cs @@ -261,8 +261,6 @@ private void SetConnectionProperties(string value) { string paramValue = Convert.ToString(builderValue)!; - this.adbcConnectionParameters.Add(key, paramValue); - switch (key) { case ConnectionStringKeywords.DecimalBehavior: @@ -276,6 +274,10 @@ private void SetConnectionProperties(string value) break; case ConnectionStringKeywords.ConnectionTimeout: this.connectionTimeoutValue = ConnectionStringParser.ParseTimeoutValue(paramValue); + this.adbcConnectionParameters.Add(connectionTimeoutValue.DriverPropertyName, connectionTimeoutValue.Value.ToString()); + break; + default: + this.adbcConnectionParameters.Add(key, paramValue); break; } } diff --git a/csharp/src/Client/ConnectionStringParser.cs b/csharp/src/Client/ConnectionStringParser.cs index 9b47246ac7..3fb602870d 100644 --- a/csharp/src/Client/ConnectionStringParser.cs +++ b/csharp/src/Client/ConnectionStringParser.cs @@ -16,9 +16,6 @@ */ using System; -using System.Collections.Generic; -using System.Net.Http.Headers; -using System.Text; using System.Text.RegularExpressions; namespace Apache.Arrow.Adbc.Client @@ -62,7 +59,7 @@ public static TimeoutValue ParseTimeoutValue(string value) } else { - throw new InvalidOperationException("cannot parse the timeout value"); + throw new ArgumentOutOfRangeException(nameof(value)); } } } diff --git a/csharp/test/Apache.Arrow.Adbc.Tests/Client/ClientTests.cs b/csharp/test/Apache.Arrow.Adbc.Tests/Client/ClientTests.cs index 0eefb811ed..e1d5a91781 100644 --- a/csharp/test/Apache.Arrow.Adbc.Tests/Client/ClientTests.cs +++ b/csharp/test/Apache.Arrow.Adbc.Tests/Client/ClientTests.cs @@ -181,7 +181,16 @@ public void TestTimeoutParsing(string value, string? driverPropertyName, int? ti { if (!success) { - Assert.Throws(() => ConnectionStringParser.ParseTimeoutValue(value)); + try + { + ConnectionStringParser.ParseTimeoutValue(value); + } + catch (ArgumentOutOfRangeException) { } + catch (InvalidOperationException) { } + catch + { + Assert.Fail("Unknown exception found"); + } } else { From ce9e71b80745bb6a1aea3d101cd3794ad08b6828 Mon Sep 17 00:00:00 2001 From: davidhcoe <13318837+davidhcoe@users.noreply.github.com> Date: Thu, 5 Dec 2024 18:33:24 -0500 Subject: [PATCH 4/5] Update csharp/src/Client/AdbcCommand.cs Co-authored-by: Bruce Irschick --- csharp/src/Client/AdbcCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csharp/src/Client/AdbcCommand.cs b/csharp/src/Client/AdbcCommand.cs index a317ca19cc..47306dba54 100644 --- a/csharp/src/Client/AdbcCommand.cs +++ b/csharp/src/Client/AdbcCommand.cs @@ -88,7 +88,7 @@ internal AdbcCommand(AdbcStatement adbcStatement, AdbcConnection adbcConnection) if (adbcConnection.CommandTimeoutValue != null) { - this.AdbcCommandTimeoutProperty = adbcConnection.CommandTimeoutValue.DriverPropertyName; + this.adbcConnectionParameters[connectionTimeoutValue.DriverPropertyName] = connectionTimeoutValue.Value.ToString(); this.CommandTimeout = adbcConnection.CommandTimeoutValue.Value; } } From e4a718021367f45aa8d1886c4e1401c1e17f3d63 Mon Sep 17 00:00:00 2001 From: Bruce Irschick Date: Thu, 5 Dec 2024 17:05:47 -0800 Subject: [PATCH 5/5] correct accidental update --- csharp/src/Client/AdbcCommand.cs | 2 +- csharp/src/Client/AdbcConnection.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/csharp/src/Client/AdbcCommand.cs b/csharp/src/Client/AdbcCommand.cs index 47306dba54..a317ca19cc 100644 --- a/csharp/src/Client/AdbcCommand.cs +++ b/csharp/src/Client/AdbcCommand.cs @@ -88,7 +88,7 @@ internal AdbcCommand(AdbcStatement adbcStatement, AdbcConnection adbcConnection) if (adbcConnection.CommandTimeoutValue != null) { - this.adbcConnectionParameters[connectionTimeoutValue.DriverPropertyName] = connectionTimeoutValue.Value.ToString(); + this.AdbcCommandTimeoutProperty = adbcConnection.CommandTimeoutValue.DriverPropertyName; this.CommandTimeout = adbcConnection.CommandTimeoutValue.Value; } } diff --git a/csharp/src/Client/AdbcConnection.cs b/csharp/src/Client/AdbcConnection.cs index c0a1af0528..a8d805f7e2 100644 --- a/csharp/src/Client/AdbcConnection.cs +++ b/csharp/src/Client/AdbcConnection.cs @@ -274,7 +274,7 @@ private void SetConnectionProperties(string value) break; case ConnectionStringKeywords.ConnectionTimeout: this.connectionTimeoutValue = ConnectionStringParser.ParseTimeoutValue(paramValue); - this.adbcConnectionParameters.Add(connectionTimeoutValue.DriverPropertyName, connectionTimeoutValue.Value.ToString()); + this.adbcConnectionParameters[connectionTimeoutValue.DriverPropertyName] = connectionTimeoutValue.Value.ToString(); break; default: this.adbcConnectionParameters.Add(key, paramValue);