CurtHagenlocher commented on code in PR #4885:
URL: https://github.com/apache/arrow-adbc/pull/4885#discussion_r4261095502
##########
csharp/src/Apache.Arrow.Adbc/C/CAdbcDriverImporter.cs:
##########
@@ -502,12 +591,87 @@ public unsafe override AdbcConnection
Connect(IReadOnlyDictionary<string, string
public unsafe override void SetOption(string key, string value)
{
- using (CallHelper caller = new CallHelper())
+ using (CallHelper caller = new CallHelper(_driver))
{
caller.Call(Driver.DatabaseSetOption, ref _nativeDatabase,
key, value);
}
}
+ public unsafe override void SetOptionInt64(string key, long value)
+ {
+ if (_driver.DriverVersion < AdbcVersion.Version_1_1_0) {
base.SetOptionInt64(key, value); return; }
+ fixed (CAdbcDatabase* database = &_nativeDatabase)
+ {
+ CAdbcDriverImporter.SetOptionInt64(_driver,
Options.SetOptionInt, database, key, value);
+ }
+ }
+
+ public unsafe override void SetOptionDouble(string key, double
value)
+ {
+ if (_driver.DriverVersion < AdbcVersion.Version_1_1_0) {
base.SetOptionDouble(key, value); return; }
+ fixed (CAdbcDatabase* database = &_nativeDatabase)
+ {
+ CAdbcDriverImporter.SetOptionDouble(_driver,
Options.SetOptionDouble, database, key, value);
+ }
+ }
+
+ public unsafe override void SetOptionBytes(string key,
ReadOnlySpan<byte> value)
+ {
+ fixed (CAdbcDatabase* database = &_nativeDatabase)
+ {
+ CAdbcDriverImporter.SetOptionBytes(_driver,
Options.SetOptionBytes, database, key, value);
+ }
+ }
Review Comment:
These calls never reach the native pointer on a 1.0 driver. `Options` goes
through `ImportedAdbcDriver.Driver11`, and `Driver11` throws
`AdbcException.NotImplemented` when the negotiated version is below 1.1.0. That
happens before any function pointer is read, so the caller gets
`NotImplemented`, the same status as the base implementation. The same applies
at lines 874 and 1526.
c38fb4f6e extends `Version1_0DriverFallsBackForTypedSetters` to check
`SetOptionBytes` at all three levels against a 1.0 driver.
##########
csharp/src/Apache.Arrow.Adbc/C/CAdbcDriverImporter.cs:
##########
@@ -606,21 +778,193 @@ public override IsolationLevel IsolationLevel
public override bool ReadOnly
{
- get => _readOnly ?? throw AdbcException.NotImplemented("no
value has been set for ReadOnly");
+ get => _readOnly ??
+ (TryGetNativeOption(AdbcOptions.Connection.ReadOnly,
AdbcOptions.GetEnabled, out bool value)
+ ? value
+ : throw AdbcException.NotImplemented("no value has
been set for ReadOnly"));
set
{
SetOption(AdbcOptions.Connection.ReadOnly,
AdbcOptions.GetEnabled(value));
_readOnly = value;
}
}
+ public override string? CurrentCatalog
+ {
+ get => GetOptionOrNull(AdbcOptions.Connection.CurrentCatalog);
+ }
+
+ public override string? CurrentDbSchema
+ {
+ get => GetOptionOrNull(AdbcOptions.Connection.CurrentDbSchema);
+ }
+
+ /// <summary>
+ /// Asks an ADBC 1.1 driver for a value that hasn't been set
through this object.
+ /// Any failure yields null so that callers keep their ADBC 1.0
behavior.
+ /// </summary>
+ private bool TryGetNativeOption<T>(string key, Func<string, T>
parse, out T value)
+ {
+ if (IsVersion1_1)
+ {
+ try
+ {
+ value = parse(GetOption(key));
+ return true;
+ }
+ catch (AdbcException)
+ {
+ }
+ catch (NotSupportedException)
+ {
+ // The driver reported a value that isn't one of the
standard ones
+ }
+ }
+
+ value = default!;
+ return false;
+ }
+
+ private string? GetOptionOrNull(string key)
+ {
+ // A 1.0 driver can't report the value. Don't call GetOption,
whose 1.0
+ // fallback reads these properties.
+ if (!IsVersion1_1)
+ {
+ return null;
+ }
+
+ try
+ {
+ return GetOption(key);
+ }
+ catch (AdbcException e) when (e.Status ==
AdbcStatusCode.NotFound || e.Status == AdbcStatusCode.NotImplemented)
+ {
+ return null;
+ }
+ }
+
+ private OptionFunctions Options
+ {
+ get
+ {
+ if (_disposed) { throw new
ObjectDisposedException(nameof(ImportedAdbcConnection)); }
+ return _driver.ConnectionOptions;
+ }
+ }
+
+ public unsafe override void SetOptionInt64(string key, long value)
+ {
+ if (!IsVersion1_1) { base.SetOptionInt64(key, value); return; }
+ fixed (CAdbcConnection* connection = &_nativeConnection)
+ {
+ CAdbcDriverImporter.SetOptionInt64(_driver,
Options.SetOptionInt, connection, key, value);
+ }
+ }
+
+ public unsafe override void SetOptionDouble(string key, double
value)
+ {
+ if (!IsVersion1_1) { base.SetOptionDouble(key, value); return;
}
+ fixed (CAdbcConnection* connection = &_nativeConnection)
+ {
+ CAdbcDriverImporter.SetOptionDouble(_driver,
Options.SetOptionDouble, connection, key, value);
+ }
+ }
+
+ public unsafe override void SetOptionBytes(string key,
ReadOnlySpan<byte> value)
+ {
+ fixed (CAdbcConnection* connection = &_nativeConnection)
+ {
+ CAdbcDriverImporter.SetOptionBytes(_driver,
Options.SetOptionBytes, connection, key, value);
+ }
+ }
+
+ public unsafe override string GetOption(string key)
+ {
+ if (_driver.DriverVersion < AdbcVersion.Version_1_1_0) {
return base.GetOption(key); }
+
+ fixed (CAdbcConnection* connection = &_nativeConnection)
+ {
+ return GetOptionString(_driver, Options.GetOption,
connection, key);
+ }
+ }
+
+ public unsafe override long GetOptionInt64(string key)
+ {
+ if (_driver.DriverVersion < AdbcVersion.Version_1_1_0) {
return base.GetOptionInt64(key); }
+
+ fixed (CAdbcConnection* connection = &_nativeConnection)
+ {
+ return CAdbcDriverImporter.GetOptionInt64(_driver,
Options.GetOptionInt, connection, key);
+ }
+ }
+
+ public unsafe override double GetOptionDouble(string key)
+ {
+ if (_driver.DriverVersion < AdbcVersion.Version_1_1_0) {
return base.GetOptionDouble(key); }
+
+ fixed (CAdbcConnection* connection = &_nativeConnection)
+ {
+ return CAdbcDriverImporter.GetOptionDouble(_driver,
Options.GetOptionDouble, connection, key);
+ }
+ }
+
+ public unsafe override byte[] GetOptionBytes(string key)
+ {
+ if (_driver.DriverVersion < AdbcVersion.Version_1_1_0) {
return base.GetOptionBytes(key); }
+
+ fixed (CAdbcConnection* connection = &_nativeConnection)
+ {
+ return GetOptionBuffer(_driver, Options.GetOptionBytes,
connection, key);
+ }
+ }
+
+ public unsafe override IArrowArrayStream GetStatistics(string?
catalogPattern, string? schemaPattern, string tableNamePattern, bool
approximate)
+ {
+ using (Utf8Helper utf8Catalog = new Utf8Helper(catalogPattern))
+ using (Utf8Helper utf8Schema = new Utf8Helper(schemaPattern))
+ using (Utf8Helper utf8Table = new Utf8Helper(tableNamePattern))
+ using (CallHelper caller = new CallHelper(_driver))
Review Comment:
Same as above: `GetStatistics` and `GetStatisticNames` read the pointer
through `Driver11`, which throws `NotImplemented` for a 1.0 driver before the
pointer is used. c38fb4f6e adds both calls to
`Version1_0DriverFallsBackForTypedSetters`.
##########
csharp/src/Apache.Arrow.Adbc/C/CAdbcDriverImporter.cs:
##########
@@ -1022,7 +1366,7 @@ public unsafe override QueryResult ExecuteQuery()
public override unsafe Schema ExecuteSchema()
{
- using (CallHelper caller = new CallHelper())
+ using (CallHelper caller = new CallHelper(_driver))
{
fixed (CAdbcStatement* statement = &_nativeStatement)
Review Comment:
`ExecuteSchema` also reads the pointer through `Driver11`, which throws
`NotImplemented` for a 1.0 driver before any native call. c38fb4f6e adds it to
`Version1_0DriverFallsBackForTypedSetters`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
