Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(csharp/src/Client): parse custom properties from connection string #2352

Merged
merged 6 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
9 changes: 8 additions & 1 deletion csharp/src/Client/AdbcCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public AdbcCommand(AdbcConnection adbcConnection) : base()

this.DbConnection = adbcConnection;
this.DecimalBehavior = adbcConnection.DecimalBehavior;
this.StructBehavior = adbcConnection.StructBehavior;
this._adbcStatement = adbcConnection.CreateStatement();
}

Expand All @@ -74,6 +75,7 @@ public AdbcCommand(string query, AdbcConnection adbcConnection) : base()

this.DbConnection = adbcConnection;
this.DecimalBehavior = adbcConnection.DecimalBehavior;
this.StructBehavior = adbcConnection.StructBehavior;
}

// For testing
Expand All @@ -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;
davidhcoe marked this conversation as resolved.
Show resolved Hide resolved
this.CommandTimeout = adbcConnection.CommandTimeoutValue.Value;
}
}

/// <summary>
Expand Down Expand Up @@ -119,7 +127,6 @@ public override CommandType CommandType
}
}


/// <summary>
/// Gets or sets the name of the command timeout property for the underlying ADBC driver.
/// </summary>
Expand Down
46 changes: 44 additions & 2 deletions csharp/src/Client/AdbcConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public sealed class AdbcConnection : DbConnection
{
private AdbcDatabase? adbcDatabase;
private Adbc.AdbcConnection? adbcConnectionInternal;
private TimeoutValue? connectionTimeoutValue;

private readonly Dictionary<string, string> adbcConnectionParameters;
private readonly Dictionary<string, string> adbcConnectionOptions;
Expand Down Expand Up @@ -122,6 +123,8 @@ internal AdbcStatement CreateStatement()
return this.adbcConnectionInternal!.CreateStatement();
}

internal TimeoutValue? CommandTimeoutValue { get; private set; }

#if NET5_0_OR_GREATER
[AllowNull]
#endif
Expand All @@ -137,11 +140,30 @@ internal AdbcStatement CreateStatement()
/// </summary>
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);
AdbcCommand cmd = new AdbcCommand(this);

if (CommandTimeoutValue != null)
{
cmd.AdbcCommandTimeoutProperty = CommandTimeoutValue.DriverPropertyName!;
cmd.CommandTimeout = CommandTimeoutValue.Value;
}

return cmd;
}

/// <summary>
Expand Down Expand Up @@ -237,7 +259,27 @@ private void SetConnectionProperties(string value)
object? builderValue = builder[key];
if (builderValue != null)
{
this.adbcConnectionParameters.Add(key, Convert.ToString(builderValue)!);
string paramValue = Convert.ToString(builderValue)!;

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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this has no effect on the underlying ADBC driver, is this to help synchronize the value with the driver?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right in that it doesn't do much. The initial driver value would get set when the properties are passed in for the driver. However, ConnectionTimeout is only a get call, and it would differ from the actual value that was passed in. That said, maybe this could be better if you have an idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a slight tweak to it so that it will set the property on the driver if it is passed in and it will also have a matching get value in that case.

this.adbcConnectionParameters.Add(connectionTimeoutValue.DriverPropertyName, connectionTimeoutValue.Value.ToString());
break;
default:
this.adbcConnectionParameters.Add(key, paramValue);
break;
}
}
}
}
Expand Down
81 changes: 81 additions & 0 deletions csharp/src/Client/ConnectionStringParser.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* 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.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 ArgumentOutOfRangeException(nameof(value));
}
}
}

internal class TimeoutValue
{
public string DriverPropertyName { get; set; } = string.Empty;

public int Value { get; set; }

// seconds=s
// milliseconds=ms
/// <remarks>
/// While these can be helpful, the DbConnection and DbCommand
/// objects limit the use of these.
/// </remarks>
public string Units { get; set; } = string.Empty;
}
}
9 changes: 9 additions & 0 deletions csharp/src/Client/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
113 changes: 113 additions & 0 deletions csharp/test/Apache.Arrow.Adbc.Tests/Client/ClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data.SqlTypes;
using System.Linq;
using System.Threading;
Expand All @@ -26,6 +27,7 @@
using Apache.Arrow.Types;
using Moq;
using Xunit;
using AdbcClient = Apache.Arrow.Adbc.Client;

namespace Apache.Arrow.Adbc.Tests.Client
{
Expand Down Expand Up @@ -166,6 +168,117 @@ 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)
{
try
{
ConnectionStringParser.ParseTimeoutValue(value);
}
catch (ArgumentOutOfRangeException) { }
catch (InvalidOperationException) { }
catch
{
Assert.Fail("Unknown exception found");
}
}
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<AdbcStatement> mockStatement = new Mock<AdbcStatement>();
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<InvalidOperationException>(() => 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; }
}

/// <summary>
/// Collection of <see cref="ConnectionStringExample"/> for testing statement timeouts."/>
/// </summary>
internal class ConnectionParsingTestData : TheoryData<ConnectionStringExample>
{
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
Expand Down
Loading