Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
David Coe committed Mar 9, 2025
1 parent df1562d commit a70b800
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 68 deletions.
95 changes: 54 additions & 41 deletions csharp/src/Apache.Arrow.Adbc/Extensions/IArrowArrayExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
using System.Collections;
using System.Collections.Generic;
using System.Data.SqlTypes;
using System.Dynamic;
using System.IO;
using System.Text.Json;
using Apache.Arrow.Types;
Expand All @@ -35,19 +34,39 @@ public enum StructResultType
public static class IArrowArrayExtensions
{
/// <summary>
/// Helper extension to get a value from the <see cref="IArrowArray"/> at the specified index.
/// Overloaded. Helper extension to get a value from the <see cref="IArrowArray"/> at the specified index.
/// </summary>
/// <param name="arrowArray">
/// The Arrow array.
/// </param>
/// <param name="index">
/// The index in the array to get the value from.
/// </param>
public static object? ValueAt(this IArrowArray arrowArray, int index)
{
return ValueAt(arrowArray, index, StructResultType.JsonString);
}

/// <summary>
/// Overloaded. Helper extension to get a value from the <see cref="IArrowArray"/> at the specified index.
/// </summary>
/// <param name="arrowArray">
/// The Arrow array.
/// </param>
/// <param name="index">
/// The index in the array to get the value from.
/// </param>
/// <param name="resultType">
/// T
/// </param>
public static object? ValueAt(this IArrowArray arrowArray, int index, StructResultType resultType = StructResultType.JsonString)
{
if (arrowArray == null) throw new ArgumentNullException(nameof(arrowArray));
if (index < 0) throw new ArgumentOutOfRangeException(nameof(index));

if (arrowArray.IsNull(index))
return null;

switch (arrowArray.Data.DataType.TypeId)
{
case ArrowTypeId.Null:
Expand Down Expand Up @@ -136,14 +155,7 @@ public static class IArrowArrayExtensions
throw new NotSupportedException($"Unsupported interval unit: {((IntervalType)arrowArray.Data.DataType).Unit}");
}
case ArrowTypeId.Binary:
if (!arrowArray.IsNull(index))
{
return ((BinaryArray)arrowArray).GetBytes(index).ToArray();
}
else
{
return null;
}
return ((BinaryArray)arrowArray).GetBytes(index).ToArray();
case ArrowTypeId.List:
return ((ListArray)arrowArray).GetSlicedValues(index);
case ArrowTypeId.Struct:
Expand All @@ -161,15 +173,29 @@ public static class IArrowArrayExtensions
}

/// <summary>
/// Helper extension to get a value from the <see cref="IArrowArray"/> at the specified index.
/// Overloaded. Helper extension to get a value converter for the <see href="IArrowType"/>.
/// </summary>
/// <param name="arrowArray">
/// The Arrow array.
/// <param name="arrayType">
/// The return type of an item in a StructArray.
/// </param>
/// <param name="index">
/// The index in the array to get the value from.
public static Func<IArrowArray, int, object?> GetValueConverter(this IArrowType arrayType)
{
return GetValueConverter(arrayType, StructResultType.JsonString);
}

/// <summary>
/// Overloaded. Helper extension to get a value from the <see cref="IArrowArray"/> at the specified index.
/// </summary>
/// <param name="arrayType">
/// The Arrow array type.
/// </param>
/// <param name="sourceType">
/// The incoming <see cref="SourceStringType"/>.
/// </param>
public static Func<IArrowArray, int, object?> GetValueConverter(this IArrowType arrayType, StructResultType resultType = StructResultType.JsonString)
/// <param name="resultType">
/// The return type of an item in a StructArray.
/// </param>
public static Func<IArrowArray, int, object?> GetValueConverter(this IArrowType arrayType, StructResultType resultType)
{
if (arrayType == null) throw new ArgumentNullException(nameof(arrayType));

Expand Down Expand Up @@ -208,23 +234,9 @@ public static class IArrowArrayExtensions
case ArrowTypeId.Int64:
return (array, index) => ((Int64Array)array).GetValue(index);
case ArrowTypeId.String:
return (array, index) =>
{
StringArray? sArray = array as StringArray;

if (sArray != null)
{
return sArray.GetString(index);
}
else
{
// some callers treat the Decimal256Array values as a string
Decimal256Array? array256 = array as Decimal256Array;
return array256?.GetString(index);
}

throw new AdbcException($"Cannot get the value at {index}. A String type was requested but neither a StringArray or Decimal256Array was found.", AdbcStatusCode.InvalidData);
};
return (array, index) => array.Data.DataType.TypeId == ArrowTypeId.Decimal256 ?
((Decimal256Array)array).GetString(index) :
((StringArray)array).GetString(index);
#if NET6_0_OR_GREATER
case ArrowTypeId.Time32:
return (array, index) => ((Time32Array)array).GetTime(index);
Expand Down Expand Up @@ -282,7 +294,9 @@ public static class IArrowArrayExtensions
case ArrowTypeId.List:
return (array, index) => ((ListArray)array).GetSlicedValues(index);
case ArrowTypeId.Struct:
return (array, index) => resultType == StructResultType.JsonString ? SerializeToJson((StructArray)array, index) : ParseStructArray((StructArray)array, index) ;
return resultType == StructResultType.JsonString ?
(array, index) => SerializeToJson((StructArray)array, index) :
(array, index) => ParseStructArray((StructArray)array, index);

// not covered:
// -- map array
Expand All @@ -299,21 +313,20 @@ public static class IArrowArrayExtensions
/// </summary>
private static string SerializeToJson(StructArray structArray, int index)
{
ExpandoObject? obj = ParseStructArray(structArray, index);
Dictionary<string, object?>? obj = ParseStructArray(structArray, index);

return JsonSerializer.Serialize(obj);
}

/// <summary>
/// Converts an item in the StructArray at the index position to an ExpandoObject.
/// Converts an item in the StructArray at the index position to a Dictionary<string, object?>.
/// </summary>
private static ExpandoObject? ParseStructArray(StructArray structArray, int index)
private static Dictionary<string, object?>? ParseStructArray(StructArray structArray, int index)
{
if (structArray.IsNull(index))
return null;

var expando = new ExpandoObject();
var jsonDictionary = (IDictionary<string, object?>)expando;
Dictionary<string, object?> jsonDictionary = new Dictionary<string, object?>();

StructType structType = (StructType)structArray.Data.DataType;
for (int i = 0; i < structArray.Data.Children.Length; i++)
Expand All @@ -323,7 +336,7 @@ private static string SerializeToJson(StructArray structArray, int index)

if (value is StructArray structArray1)
{
List<ExpandoObject?> children = new List<ExpandoObject?>();
List<Dictionary<string, object?>?> children = new List<Dictionary<string, object?>?>();

for (int j = 0; j < structArray1.Length; j++)
{
Expand Down Expand Up @@ -363,7 +376,7 @@ private static string SerializeToJson(StructArray structArray, int index)
}
}

return expando;
return jsonDictionary;
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions csharp/src/Client/SchemaConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
*/

using System;
using System.Collections.Generic;
using System.Data;
using System.Data.Common;
using System.Data.SqlTypes;
using System.Dynamic;
using Apache.Arrow.Scalars;
using Apache.Arrow.Types;

Expand Down Expand Up @@ -194,7 +194,7 @@ public static Type GetArrowType(Field f, DecimalBehavior decimalBehavior, Struct
return typeof(string);

case ArrowTypeId.Struct:
return structBehavior == StructBehavior.JsonString ? typeof(string) : typeof(ExpandoObject);
return structBehavior == StructBehavior.JsonString ? typeof(string) : typeof(Dictionary<string, object?>);

case ArrowTypeId.Timestamp:
return typeof(DateTimeOffset);
Expand Down
2 changes: 1 addition & 1 deletion csharp/src/Client/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,5 @@ 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). If using JsonString, the return ArrowType will be StringType and a string value. If using Strict, the return ArrowType will be StructType and an ExpandoObject.
- __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). If using JsonString, the return ArrowType will be StringType and the result a string value. If using Strict, the return ArrowType will be StructType and the result a Dictionary<string, object?>.
- __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.
5 changes: 3 additions & 2 deletions csharp/src/Drivers/BigQuery/BigQueryStatement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ sealed class ReadRowsStream : Stream
ReadOnlyMemory<byte> currentBuffer;
bool first;
int position;
bool hasRows = true;
bool hasRows;

public ReadRowsStream(IAsyncEnumerator<ReadRowsResponse> response)
{
Expand All @@ -383,6 +383,7 @@ public ReadRowsStream(IAsyncEnumerator<ReadRowsResponse> response)
if (response.Current != null)
{
this.currentBuffer = response.Current.ArrowSchema.SerializedSchema.Memory;
this.hasRows = true;
}
else
{
Expand All @@ -393,7 +394,7 @@ public ReadRowsStream(IAsyncEnumerator<ReadRowsResponse> response)
this.first = true;
}

public bool HasRows { get => this.hasRows; }
public bool HasRows => this.hasRows;

public override bool CanRead => true;

Expand Down
26 changes: 10 additions & 16 deletions csharp/test/Apache.Arrow.Adbc.Tests/ClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
using System.Collections.ObjectModel;
using System.Data;
using System.Data.Common;
using System.Dynamic;
using System.Linq;
using Apache.Arrow.Adbc.Client;
using Apache.Arrow.Types;
Expand Down Expand Up @@ -279,15 +278,15 @@ static void AssertTypeAndValue(
{
bool areEqual = false;

if (value is ExpandoObject)
if (value is Dictionary<string, object?>)
{
if (value == null && ctv.ExpectedValue == null)
{
areEqual = true;
}
else
{
areEqual = AreExpandoObjectsEqual(value as ExpandoObject, ctv.ExpectedValue as ExpandoObject);
areEqual = AreDictionariesEqual(value as Dictionary<string, object?>, ctv.ExpectedValue as Dictionary<string, object?>);
}
}
else
Expand Down Expand Up @@ -332,39 +331,34 @@ static void AssertTypeAndValue(
}
}

static bool AreExpandoObjectsEqual(ExpandoObject? obj1, ExpandoObject? obj2)
static bool AreDictionariesEqual(Dictionary<string, object?>? dict1, Dictionary<string, object?>? dict2)
{
if (obj1 == null && obj2 == null)
if (dict1 == null && dict2 == null)
{
return true;
}
else if (obj1 != null && obj2 == null)
else if (dict1 != null && dict2 == null)
{
return false;
}
else if (obj1 == null && obj2 != null)
else if (dict1 == null && dict2 != null)
{
return false;
}

var dict1 = (IDictionary<string, object?>)obj1!;
var dict2 = (IDictionary<string, object?>)obj2!;

if (dict1.Count != dict2.Count)
if (dict1!.Count != dict2!.Count)
return false;

foreach (var key in dict1.Keys)
{
if (!dict2.ContainsKey(key))
if (!dict2.TryGetValue(key, out object? value2))
return false;

object? value1 = dict1[key];
object? value2 = dict2[key];

if (value1 is ExpandoObject expando1 && value2 is ExpandoObject expando2)
if (value1 is Dictionary<string, object?> nextObj1 && value2 is Dictionary<string, object?> nextObj2)
{
// Recursively compare nested ExpandoObjects
if (!AreExpandoObjectsEqual(expando1, expando2))
if (!AreDictionariesEqual(nextObj1, nextObj2))
return false;
}
else if (!object.Equals(value1, value2))
Expand Down
11 changes: 5 additions & 6 deletions csharp/test/Drivers/BigQuery/BigQueryData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,9 @@ public static SampleDataBuilder GetSampleData()

SampleDataBuilder sampleDataBuilder = new SampleDataBuilder();

ExpandoObject person = new ExpandoObject();
IDictionary<string, object?> keyValues = (IDictionary<string, object?>)person;
keyValues["name"] = "John Doe";
keyValues["age"] = 30L;
Dictionary<string, object?> person = new Dictionary<string, object?>();
person["name"] = "John Doe";
person["age"] = 30L;

// StructBehavior = "Strict"
sampleDataBuilder.Samples.Add(
Expand All @@ -58,7 +57,7 @@ public static SampleDataBuilder GetSampleData()
StructBehavior = "Strict",
ExpectedValues = new List<ColumnNetTypeArrowTypeValue>()
{
new ColumnNetTypeArrowTypeValue("person", typeof(ExpandoObject), typeof(StructType), person),
new ColumnNetTypeArrowTypeValue("person", typeof(Dictionary<string, object?>), typeof(StructType), person),
}
});

Expand Down Expand Up @@ -116,7 +115,7 @@ public static SampleDataBuilder GetSampleData()
new ColumnNetTypeArrowTypeValue("point", typeof(string), typeof(StringType), "POINT(1 2)"),
new ColumnNetTypeArrowTypeValue("numbers", typeof(Int64Array), typeof(ListType), numbersArray),

new ColumnNetTypeArrowTypeValue("person", typeof(ExpandoObject), typeof(StructType), person),
new ColumnNetTypeArrowTypeValue("person", typeof(Dictionary<string, object?>), typeof(StructType), person),
new ColumnNetTypeArrowTypeValue("json", typeof(string), typeof(StringType), "{\"age\":29,\"name\":\"Jane Doe\"}")
}
});
Expand Down

0 comments on commit a70b800

Please sign in to comment.