Skip to content

Commit

Permalink
feat(csharp/src/Apache.Arrow.Adbc): Cleanup use of List<T> in APIs an…
Browse files Browse the repository at this point in the history
…d implementation (apache#1761)

Replaces the use of `List<T>` in most API signatures with
`IReadOnlyList<T>` to allow many different types of collections (such as
arrays) to be passed in.

Replaces the use of `List<T>` in many implementations with `T[]` for
performance and readability benefits.

Closes apache#1757
  • Loading branch information
CurtHagenlocher authored and cocoa-xu committed May 8, 2024
1 parent 11d4aa0 commit 19dea92
Show file tree
Hide file tree
Showing 13 changed files with 128 additions and 105 deletions.
4 changes: 2 additions & 2 deletions csharp/src/Apache.Arrow.Adbc/AdbcConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public virtual void Dispose()
/// Metadata about the driver and/or database
/// </returns>
/// <exception cref="ArgumentNullException"></exception>
public virtual IArrowArrayStream GetInfo(List<AdbcInfoCode> codes)
public virtual IArrowArrayStream GetInfo(IReadOnlyList<AdbcInfoCode> codes)
{
if (codes == null)
throw new ArgumentNullException(nameof(codes));
Expand Down Expand Up @@ -119,7 +119,7 @@ public abstract IArrowArrayStream GetObjects(
string catalogPattern,
string dbSchemaPattern,
string tableNamePattern,
List<string> tableTypes,
IReadOnlyList<string> tableTypes,
string columnNamePattern);

public enum GetObjectsDepth
Expand Down
9 changes: 4 additions & 5 deletions csharp/src/Apache.Arrow.Adbc/C/CAdbcDriverExporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
using Apache.Arrow.C;
using Apache.Arrow.Ipc;


#if NETSTANDARD
using Apache.Arrow.Adbc.Extensions;
#endif
Expand Down Expand Up @@ -775,7 +774,7 @@ public unsafe void GetObjects(ref CAdbcConnection nativeConnection, int depth, b
columnNamePattern = Marshal.PtrToStringUTF8((IntPtr)column_name);
#endif

List<string> tableTypes = null;
string[] tableTypes = null;
const int maxTableTypeCount = 100;
if (table_type != null)
{
Expand All @@ -790,13 +789,13 @@ public unsafe void GetObjects(ref CAdbcConnection nativeConnection, int depth, b
throw new InvalidOperationException($"We do not expect to get more than {maxTableTypeCount} table types");
}

tableTypes = new List<string>(count);
tableTypes = new string[count];
for (int i = 0; i < count; i++)
{
#if NETSTANDARD
tableTypes.Add(MarshalExtensions.PtrToStringUTF8((IntPtr)table_type[i]));
tableTypes[i] = MarshalExtensions.PtrToStringUTF8((IntPtr)table_type[i]);
#else
tableTypes.Add(Marshal.PtrToStringUTF8((IntPtr)table_type[i]));
tableTypes[i] = Marshal.PtrToStringUTF8((IntPtr)table_type[i]);
#endif
}
}
Expand Down
20 changes: 9 additions & 11 deletions csharp/src/Apache.Arrow.Adbc/C/CAdbcDriverImporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using Apache.Arrow.Adbc.Extensions;
using Apache.Arrow.C;
using Apache.Arrow.Ipc;

#if NETSTANDARD
using Apache.Arrow.Adbc.Extensions;
#endif

namespace Apache.Arrow.Adbc.C
{
Expand Down Expand Up @@ -222,7 +220,7 @@ public unsafe override AdbcStatement CreateStatement()
return new AdbcStatementNative(_nativeDriver, nativeStatement);
}

public unsafe override IArrowArrayStream GetInfo(List<AdbcInfoCode> codes)
public unsafe override IArrowArrayStream GetInfo(IReadOnlyList<AdbcInfoCode> codes)
{
CArrowArrayStream* nativeArrayStream = CArrowArrayStream.Create();

Expand All @@ -236,7 +234,7 @@ public unsafe override IArrowArrayStream GetInfo(List<AdbcInfoCode> codes)
return arrowArrayStream;
}

public override unsafe IArrowArrayStream GetObjects(GetObjectsDepth depth, string catalogPattern, string dbSchemaPattern, string tableNamePattern, List<string> tableTypes, string columnNamePattern)
public override unsafe IArrowArrayStream GetObjects(GetObjectsDepth depth, string catalogPattern, string dbSchemaPattern, string tableNamePattern, IReadOnlyList<string> tableTypes, string columnNamePattern)
{
CArrowArrayStream* nativeArrayStream = CArrowArrayStream.Create();

Expand Down Expand Up @@ -691,20 +689,20 @@ public unsafe void Dispose()
}

#if NET5_0_OR_GREATER
public unsafe void Call(delegate* unmanaged<CAdbcConnection*, int*, int, CArrowArrayStream*, CAdbcError*, AdbcStatusCode> fn, ref CAdbcConnection connection, List<AdbcInfoCode> infoCodes, CArrowArrayStream* stream)
public unsafe void Call(delegate* unmanaged<CAdbcConnection*, int*, int, CArrowArrayStream*, CAdbcError*, AdbcStatusCode> fn, ref CAdbcConnection connection, IReadOnlyList<AdbcInfoCode> infoCodes, CArrowArrayStream* stream)
{
fixed (CAdbcConnection* cn = &connection)
fixed (CAdbcError* e = &_error)
{
Span<AdbcInfoCode> span = CollectionsMarshal.AsSpan(infoCodes);
Span<AdbcInfoCode> span = infoCodes.AsSpan();
fixed (AdbcInfoCode* spanPtr = span)
{
TranslateCode(fn(cn, (int*)spanPtr, infoCodes.Count, stream, e));
}
}
}
#else
public unsafe void Call(IntPtr ptr, ref CAdbcConnection connection, List<AdbcInfoCode> infoCodes, CArrowArrayStream* stream)
public unsafe void Call(IntPtr ptr, ref CAdbcConnection connection, IReadOnlyList<AdbcInfoCode> infoCodes, CArrowArrayStream* stream)
{
fixed (CAdbcConnection* cn = &connection)
fixed (CAdbcError* e = &_error)
Expand All @@ -719,16 +717,16 @@ public unsafe void Call(IntPtr ptr, ref CAdbcConnection connection, List<AdbcInf
#endif

#if NET5_0_OR_GREATER
public unsafe void Call(delegate* unmanaged<CAdbcConnection*, int, byte*, byte*, byte*, byte**, byte*, CArrowArrayStream*, CAdbcError*, AdbcStatusCode> fn, ref CAdbcConnection connection, int depth, string catalog, string db_schema, string table_name, List<string> table_types, string column_name, CArrowArrayStream* stream)
public unsafe void Call(delegate* unmanaged<CAdbcConnection*, int, byte*, byte*, byte*, byte**, byte*, CArrowArrayStream*, CAdbcError*, AdbcStatusCode> fn, ref CAdbcConnection connection, int depth, string catalog, string db_schema, string table_name, IReadOnlyList<string> table_types, string column_name, CArrowArrayStream* stream)
#else
public unsafe void Call(IntPtr fn, ref CAdbcConnection connection, int depth, string catalog, string db_schema, string table_name, List<string> table_types, string column_name, CArrowArrayStream* stream)
public unsafe void Call(IntPtr fn, ref CAdbcConnection connection, int depth, string catalog, string db_schema, string table_name, IReadOnlyList<string> table_types, string column_name, CArrowArrayStream* stream)
#endif
{
byte* bcatalog, bDb_schema, bTable_name, bColumn_Name;

if (table_types == null)
{
table_types = new List<string>();
table_types = new string[0];
}

// need to terminate with a null entry per https://github.com/apache/arrow-adbc/blob/b97e22c4d6524b60bf261e1970155500645be510/adbc.h#L909-L911
Expand Down
43 changes: 43 additions & 0 deletions csharp/src/Apache.Arrow.Adbc/Extensions/CollectionExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* 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.Linq;

#if NET5_0_OR_GREATER
using System.Runtime.InteropServices;
#endif

namespace Apache.Arrow.Adbc.Extensions
{
internal static class CollectionExtensions
{
public static Span<T> AsSpan<T>(this IReadOnlyList<T> list)
{
T[] array = list as T[];
if (array != null) { return array.AsSpan(); }

#if NET5_0_OR_GREATER
List<T> concreteList = list as List<T>;
if (concreteList != null) { return CollectionsMarshal.AsSpan(concreteList); }
#endif

return list.ToArray().AsSpan();
}
}
}
6 changes: 3 additions & 3 deletions csharp/src/Apache.Arrow.Adbc/Results.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ public sealed class PartitionedResult
{
private readonly Schema _schema;
private readonly long _affectedRows = -1;
private readonly List<PartitionDescriptor> _partitionDescriptors;
private readonly IReadOnlyList<PartitionDescriptor> _partitionDescriptors;

public PartitionedResult(Schema schema, long affectedRows, List<PartitionDescriptor> partitionDescriptors)
public PartitionedResult(Schema schema, long affectedRows, IReadOnlyList<PartitionDescriptor> partitionDescriptors)
{
_schema = schema;
_affectedRows = affectedRows;
Expand All @@ -98,6 +98,6 @@ public PartitionedResult(Schema schema, long affectedRows, List<PartitionDescrip
/// <summary>
/// Get partitions.
/// </summary>
public List<PartitionDescriptor> PartitionDescriptors { get => _partitionDescriptors; }
public IReadOnlyList<PartitionDescriptor> PartitionDescriptors { get => _partitionDescriptors; }
}
}
23 changes: 11 additions & 12 deletions csharp/src/Apache.Arrow.Adbc/StandardSchemas.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* limitations under the License.
*/

using System;
using System.Collections.Generic;
using System.Linq;
using Apache.Arrow.Types;
Expand All @@ -33,13 +32,13 @@ public static class StandardSchemas
/// </summary>
public static readonly Schema GetInfoSchema =
new Schema(
new List<Field>()
new Field[]
{
new Field("info_name", UInt32Type.Default, false),
new Field(
"info_value",
new UnionType(
new List<Field>()
new Field[]
{
new Field("string_value", StringType.Default, true),
new Field("bool_value", BooleanType.Default, true),
Expand All @@ -56,7 +55,7 @@ public static class StandardSchemas
"int32_to_int32_list_map",
new ListType(
new Field("entries", new StructType(
new List<Field>()
new Field[]
{
new Field("key", Int32Type.Default, false),
new Field("value", Int32Type.Default, true),
Expand All @@ -75,22 +74,22 @@ public static class StandardSchemas
);

public static readonly Schema TableTypesSchema = new Schema(
new List<Field>()
new Field[]
{
new Field("table_type", StringType.Default, false)
},
metadata: null
);

public static readonly List<Field> UsageSchema = new List<Field>()
public static readonly IReadOnlyList<Field> UsageSchema = new Field[]
{
new Field("fk_catalog", StringType.Default, true),
new Field("fk_db_schema", StringType.Default, true),
new Field("fk_table", StringType.Default, false),
new Field("fk_column_name", StringType.Default, false)
};

public static readonly List<Field> ConstraintSchema = new List<Field>()
public static readonly IReadOnlyList<Field> ConstraintSchema = new Field[]
{
new Field("constraint_name", StringType.Default, false),
new Field("constraint_type", StringType.Default, false),
Expand All @@ -108,8 +107,8 @@ public static class StandardSchemas
),
};

public static readonly List<Field> ColumnSchema =
new List<Field>()
public static readonly IReadOnlyList<Field> ColumnSchema =
new Field[]
{
new Field("column_name", StringType.Default, false),
new Field("ordinal_position", Int32Type.Default, true),
Expand All @@ -132,7 +131,7 @@ public static class StandardSchemas
new Field("xdbc_is_generatedcolumn", StringType.Default, true)
};

public static readonly List<Field> TableSchema = new List<Field>() {
public static readonly IReadOnlyList<Field> TableSchema = new Field[] {
new Field("table_name", StringType.Default, false, null),
new Field("table_type", StringType.Default, false, null),
new Field(
Expand All @@ -151,7 +150,7 @@ public static class StandardSchemas
)
};

public static readonly List<Field> DbSchemaSchema = new List<Field>()
public static readonly IReadOnlyList<Field> DbSchemaSchema = new Field[]
{
new Field("db_schema_name", StringType.Default, false, null),
new Field(
Expand All @@ -164,7 +163,7 @@ public static class StandardSchemas
};

public static readonly Schema GetObjectsSchema = new Schema(
new List<Field>()
new Field[]
{
new Field("catalog_name", StringType.Default, false),
new Field(
Expand Down
2 changes: 1 addition & 1 deletion csharp/src/Drivers/Apache/Hive2/HiveServer2Connection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public void Open()
protected abstract TProtocol CreateProtocol();
protected abstract TOpenSessionReq CreateSessionRequest();

public override IArrowArrayStream GetObjects(GetObjectsDepth depth, string catalogPattern, string dbSchemaPattern, string tableNamePattern, List<string> tableTypes, string columnNamePattern)
public override IArrowArrayStream GetObjects(GetObjectsDepth depth, string catalogPattern, string dbSchemaPattern, string tableNamePattern, IReadOnlyList<string> tableTypes, string columnNamePattern)
{
throw new NotImplementedException();
}
Expand Down
2 changes: 1 addition & 1 deletion csharp/src/Drivers/Apache/Impala/ImpalaConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public override AdbcStatement CreateStatement()
return new ImpalaStatement(this);
}

public override IArrowArrayStream GetObjects(GetObjectsDepth depth, string catalogPattern, string dbSchemaPattern, string tableNamePattern, List<string> tableTypes, string columnNamePattern)
public override IArrowArrayStream GetObjects(GetObjectsDepth depth, string catalogPattern, string dbSchemaPattern, string tableNamePattern, IReadOnlyList<string> tableTypes, string columnNamePattern)
{
throw new System.NotImplementedException();
}
Expand Down
Loading

0 comments on commit 19dea92

Please sign in to comment.