[PR] feat(csharp): support extension types in the ADO.NET client [arrow-adbc]

["CurtHagenlocher (via GitHub)" <gi...@apache.org>]

CurtHagenlocher opened a new pull request, #4919:
URL: https://github.com/apache/arrow-adbc/pull/4919

   ## What's changed
   
   The ADO.NET client now reads and binds the canonical extension types that Apache.Arrow 23 implements.
   
   | Extension | .NET value | Binding |
   |---|---|---|
   | `arrow.uuid` | `Guid` | `DbType.Guid` (from a `Guid`, a string, or 16 bytes) |
   | `arrow.bool8` | `bool` | — |
   | `arrow.timestamp_with_offset` | `DateTimeOffset`, keeping the original offset | — |
   | `arrow.parquet.variant` | JSON string or `Dictionary`/`List`/scalar, depending on `StructBehavior` | — |
   | any other extension type | the storage value | — |
   
   Previously, any extension column made the reader throw `NotSupportedException`.
   
   ### Drivers loaded through the C API
   
   `CArrowArrayStreamImporter` doesn't take an extension registry, so a native driver's UUID column arrives as `fixed_size_binary(16)` with only `ARROW:extension:name` in its field metadata. The reader resolves these fields using the built-in definitions plus anything registered in `ExtensionTypeRegistry.Default`, then wraps the storage column in the extension array the first time it's read in each batch. The new `AdbcDataReader.GetArrowArray(ordinal)` returns that wrapped array. Once apache/arrow-dotnet#466 adds a registry overload to the stream importer, this becomes a fallback.
   
   ### Other changes
   
   - `IArrowArrayExtensions.ValueAt` and `GetValueConverter` handle `GuidArray`, `Bool8Array`, `TimestampWithOffsetArray`, and fall back to storage for other extension arrays. They also now support `fixed_size_binary`.
   - `Prepare()` maps uuid, bool8 and timestamp_with_offset parameters to `DbType.Guid`, `DbType.Boolean` and `DbType.DateTimeOffset`.
   - `DbType.DateTimeOffset` parameters are bound as UTC timestamps, which every driver understands; the original offset is not sent.
   - The client now references `Apache.Arrow.Operations` to read shredded variants and to format variants as JSON. The core library has no new dependency.
   - The client readme documents the mappings.
   
   ### Tests
   
   - Mocked-driver tests cover each type, both as real extension arrays and as storage with extension metadata, plus user-registered definitions, unknown extension types, parameter binding and `Prepare()`.
   - A DuckDB test with `arrow_lossless_conversion` reads UUID and BOOLEAN as `Guid` and `bool` and round-trips `Guid` parameters. DuckDB 1.5.6 can't export VARIANT over Arrow, so variants are only covered by the mocked tests.
   
   Closes #4918.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(csharp): support extension types in the ADO.NET client [arrow-adbc]

["CurtHagenlocher (via GitHub)" <gi...@apache.org>]

CurtHagenlocher merged PR #4919:
URL: https://github.com/apache/arrow-adbc/pull/4919


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(csharp): support extension types in the ADO.NET client [arrow-adbc]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #4919:
URL: https://github.com/apache/arrow-adbc/pull/4919#discussion_r4307720726


##########
csharp/src/Client/AdbcDataReader.cs:
##########
@@ -244,6 +257,27 @@ public override string GetName(int ordinal)
             return this.schema.GetFieldByIndex(ordinal)?.Name ?? string.Empty;
         }
 
+        /// <summary>
+        /// Gets the Arrow array for a column in the current record batch. Columns with an
+        /// extension type known only from field metadata are wrapped in the extension array.
+        /// </summary>
+        /// <param name="ordinal">The zero-based column ordinal.</param>
+        public IArrowArray GetArrowArray(int ordinal)
+        {
+            IArrowArray? column = this.columns[ordinal];
+            if (column == null)
+            {
+                column = this.RecordBatch.Column(ordinal);

Review Comment:
   After Read() and GetValue(0), Close()/Dispose() releases the record batch but leaves the cached column populated. Subsequent GetValue() or GetArrowArray() calls bypass the RecordBatch check and access a disposed array, potentially backed by released native memory. Validate RecordBatch before consulting the cache, and add a read/access/close/access regression test.



##########
csharp/src/Client/ExtensionTypes.cs:
##########
@@ -0,0 +1,186 @@
+/*
+* 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 Apache.Arrow.Adbc.Extensions;
+using Apache.Arrow.Operations.Shredding;
+using Apache.Arrow.Operations.VariantJson;
+using Apache.Arrow.Scalars.Variant;
+using Apache.Arrow.Types;
+
+namespace Apache.Arrow.Adbc.Client
+{
+    /// <summary>
+    /// Support for Arrow extension types in the ADO.NET client.
+    /// </summary>
+    internal static class ExtensionTypes
+    {
+        internal const string ExtensionNameKey = "ARROW:extension:name";
+        internal const string ExtensionMetadataKey = "ARROW:extension:metadata";
+
+        private static readonly DateTimeOffset s_unixEpoch = new DateTimeOffset(1970, 1, 1, 0, 0, 0, TimeSpan.Zero);
+
+        /// <summary>
+        /// The extension types the client understands without any registration by the caller.
+        /// </summary>
+        private static readonly ExtensionTypeRegistry s_builtIn = CreateBuiltInRegistry();
+
+        private static ExtensionTypeRegistry CreateBuiltInRegistry()
+        {
+            ExtensionTypeRegistry registry = new ExtensionTypeRegistry();
+            registry.Register(GuidExtensionDefinition.Instance);
+            registry.Register(Bool8ExtensionDefinition.Instance);
+            registry.Register(TimestampWithOffsetExtensionDefinition.Instance);
+            registry.Register(VariantExtensionDefinition.Instance);
+            return registry;
+        }
+
+        /// <summary>
+        /// Arrays imported through the C Data Interface carry extension types only as field
+        /// metadata over their storage type. Returns a field whose type is the corresponding
+        /// <see cref="ExtensionType"/> when the extension is known, or the original field otherwise.
+        /// Definitions registered with <see cref="ExtensionTypeRegistry.Default"/> take precedence
+        /// over the built-in ones.
+        /// </summary>
+        public static Field Resolve(Field field)
+        {
+            if (field.DataType is ExtensionType ||
+                !field.HasMetadata ||
+                !field.Metadata.TryGetValue(ExtensionNameKey, out string? name))
+            {
+                return field;

Review Comment:
   Extension metadata is resolved only on top-level fields. For a native struct with an arrow.uuid child stored as fixed_size_binary(16), the parent returns unchanged here. ParseStructArray then returns byte[] for that child under StructBehavior.Strict, while the equivalent managed struct containing a GuidArray returns Guid. Resolve nested fields and wrap their storage arrays too; add a regression test for a UUID child inside a struct.



##########
csharp/src/Client/AdbcCommand.cs:
##########
@@ -382,6 +395,17 @@ private void BindParameters()
                             }
                             parameters[i] = doubleBuilder.Build();
                             break;
+                        case DbType.Guid:
+                            var guidBuilder = new GuidArray.Builder();
+                            switch (param.Value)
+                            {
+                                case null: guidBuilder.AppendNull(); break;

Review Comment:
   Both new binding branches reject Value = DBNull.Value, the standard ADO.NET database-null representation. AdbcParameter.Value passes it through unchanged, so Guid.Parse or DateTimeOffset.Parse fails and ConvertValue throws NotSupportedException instead of binding SQL NULL. Handle DBNull alongside null in both the Guid and DateTimeOffset cases, keep the resulting field nullable, and test both null parameters.



##########
csharp/src/Client/ExtensionTypes.cs:
##########
@@ -0,0 +1,186 @@
+/*
+* 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 Apache.Arrow.Adbc.Extensions;
+using Apache.Arrow.Operations.Shredding;
+using Apache.Arrow.Operations.VariantJson;
+using Apache.Arrow.Scalars.Variant;
+using Apache.Arrow.Types;
+
+namespace Apache.Arrow.Adbc.Client
+{
+    /// <summary>
+    /// Support for Arrow extension types in the ADO.NET client.
+    /// </summary>
+    internal static class ExtensionTypes
+    {
+        internal const string ExtensionNameKey = "ARROW:extension:name";
+        internal const string ExtensionMetadataKey = "ARROW:extension:metadata";
+
+        private static readonly DateTimeOffset s_unixEpoch = new DateTimeOffset(1970, 1, 1, 0, 0, 0, TimeSpan.Zero);
+
+        /// <summary>
+        /// The extension types the client understands without any registration by the caller.
+        /// </summary>
+        private static readonly ExtensionTypeRegistry s_builtIn = CreateBuiltInRegistry();
+
+        private static ExtensionTypeRegistry CreateBuiltInRegistry()
+        {
+            ExtensionTypeRegistry registry = new ExtensionTypeRegistry();
+            registry.Register(GuidExtensionDefinition.Instance);
+            registry.Register(Bool8ExtensionDefinition.Instance);
+            registry.Register(TimestampWithOffsetExtensionDefinition.Instance);
+            registry.Register(VariantExtensionDefinition.Instance);
+            return registry;
+        }
+
+        /// <summary>
+        /// Arrays imported through the C Data Interface carry extension types only as field
+        /// metadata over their storage type. Returns a field whose type is the corresponding
+        /// <see cref="ExtensionType"/> when the extension is known, or the original field otherwise.
+        /// Definitions registered with <see cref="ExtensionTypeRegistry.Default"/> take precedence
+        /// over the built-in ones.
+        /// </summary>
+        public static Field Resolve(Field field)
+        {
+            if (field.DataType is ExtensionType ||
+                !field.HasMetadata ||
+                !field.Metadata.TryGetValue(ExtensionNameKey, out string? name))
+            {
+                return field;
+            }
+
+            if (!ExtensionTypeRegistry.Default.TryGetDefinition(name, out ExtensionDefinition definition) &&
+                !s_builtIn.TryGetDefinition(name, out definition))
+            {
+                return field;
+            }
+
+            field.Metadata.TryGetValue(ExtensionMetadataKey, out string? metadata);
+            if (!definition.TryCreateType(field.DataType, metadata ?? string.Empty, out ExtensionType extensionType))
+            {
+                return field;
+            }
+
+            return new Field(field.Name, extensionType, field.IsNullable, field.Metadata);
+        }
+
+        /// <summary>
+        /// Gets a value converter for the given type, adding the client's handling of
+        /// variants to the conversions supplied by <see cref="IArrowArrayExtensions"/>.
+        /// </summary>
+        public static Func<IArrowArray, int, object?> GetValueConverter(IArrowType type, StructBehavior structBehavior)
+        {
+            if (type is VariantType)
+            {
+                if (structBehavior == StructBehavior.JsonString)
+                {
+                    return (array, index) => array.IsNull(index) ? null : VariantJsonWriter.ToJson(GetVariantValue((VariantArray)array, index));
+                }
+                return (array, index) => array.IsNull(index) ? null : ToObject(GetVariantValue((VariantArray)array, index));
+            }
+
+            StructResultType structResultType = structBehavior == StructBehavior.JsonString ? StructResultType.JsonString : StructResultType.Object;
+            return type.GetValueConverter(structResultType);
+        }
+
+        private static VariantValue GetVariantValue(VariantArray array, int index)
+        {
+            return array.IsShredded ? array.GetLogicalVariantValue(index) : array.GetVariantValue(index);
+        }
+
+        /// <summary>
+        /// Converts a variant value to the equivalent .NET value. Objects become
+        /// <see cref="Dictionary{TKey, TValue}"/> and arrays become <see cref="List{T}"/>.
+        /// </summary>
+        internal static object? ToObject(VariantValue value)
+        {
+            if (value.IsObject)
+            {
+                Dictionary<string, object?> result = new Dictionary<string, object?>();
+                foreach (KeyValuePair<string, VariantValue> field in value.AsObject())
+                {
+                    result.Add(field.Key, ToObject(field.Value));
+                }
+                return result;
+            }
+
+            if (value.IsArray)
+            {
+                IReadOnlyList<VariantValue> elements = value.AsArray();
+                List<object?> result = new List<object?>(elements.Count);
+                foreach (VariantValue element in elements)
+                {
+                    result.Add(ToObject(element));
+                }
+                return result;
+            }
+
+            switch (value.PrimitiveType)
+            {
+                case VariantPrimitiveType.NullType:
+                    return null;
+                case VariantPrimitiveType.BooleanTrue:
+                case VariantPrimitiveType.BooleanFalse:
+                    return value.AsBoolean();
+                case VariantPrimitiveType.Int8:
+                    return value.AsInt8();
+                case VariantPrimitiveType.Int16:
+                    return value.AsInt16();
+                case VariantPrimitiveType.Int32:
+                    return value.AsInt32();
+                case VariantPrimitiveType.Int64:
+                    return value.AsInt64();
+                case VariantPrimitiveType.Float:
+                    return value.AsFloat();
+                case VariantPrimitiveType.Double:
+                    return value.AsDouble();
+                case VariantPrimitiveType.Decimal4:
+                case VariantPrimitiveType.Decimal8:
+                    return value.AsDecimal();
+                case VariantPrimitiveType.Decimal16:
+                    return value.AsSqlDecimal();
+                case VariantPrimitiveType.Date:
+                    return value.AsDate();
+                case VariantPrimitiveType.Timestamp:
+                    return value.AsTimestamp();
+                case VariantPrimitiveType.TimestampNtz:
+                    return value.AsTimestampNtz();
+                case VariantPrimitiveType.TimestampTzNanos:
+                    return s_unixEpoch.AddTicks(value.AsTimestampTzNanos() / 100);
+                case VariantPrimitiveType.TimestampNtzNanos:
+                    return s_unixEpoch.UtcDateTime.AddTicks(value.AsTimestampNtzNanos() / 100);

Review Comment:
   Under StructBehavior.Strict, TimestampNtzNanos becomes a DateTime with Kind=Utc because UtcDateTime supplies a UTC epoch and AddTicks preserves its kind. This labels a timezone-less value as a UTC instant, affecting serialization and timezone conversions. Use an unspecified-kind epoch and add a test for the returned DateTime.Kind.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(csharp): support extension types in the ADO.NET client [arrow-adbc]

["CurtHagenlocher (via GitHub)" <gi...@apache.org>]

CurtHagenlocher commented on code in PR #4919:
URL: https://github.com/apache/arrow-adbc/pull/4919#discussion_r4307918794


##########
csharp/src/Client/ExtensionTypes.cs:
##########
@@ -0,0 +1,259 @@
+/*
+* 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 Apache.Arrow.Adbc.Extensions;
+using Apache.Arrow.Operations.Shredding;
+using Apache.Arrow.Operations.VariantJson;
+using Apache.Arrow.Scalars.Variant;
+using Apache.Arrow.Types;
+
+namespace Apache.Arrow.Adbc.Client
+{
+    /// <summary>
+    /// Support for Arrow extension types in the ADO.NET client.
+    /// </summary>
+    internal static class ExtensionTypes
+    {
+        internal const string ExtensionNameKey = "ARROW:extension:name";
+        internal const string ExtensionMetadataKey = "ARROW:extension:metadata";
+
+        private static readonly DateTimeOffset s_unixEpoch = new DateTimeOffset(1970, 1, 1, 0, 0, 0, TimeSpan.Zero);
+        private static readonly DateTime s_unixEpochUnspecified = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Unspecified);
+
+        /// <summary>
+        /// The extension types the client understands without any registration by the caller.
+        /// </summary>
+        private static readonly ExtensionTypeRegistry s_builtIn = CreateBuiltInRegistry();
+
+        private static ExtensionTypeRegistry CreateBuiltInRegistry()
+        {
+            ExtensionTypeRegistry registry = new ExtensionTypeRegistry();
+            registry.Register(GuidExtensionDefinition.Instance);
+            registry.Register(Bool8ExtensionDefinition.Instance);
+            registry.Register(TimestampWithOffsetExtensionDefinition.Instance);
+            registry.Register(VariantExtensionDefinition.Instance);
+            return registry;
+        }
+
+        /// <summary>
+        /// Arrays imported through the C Data Interface carry extension types only as field
+        /// metadata over their storage type. Returns a field whose type is the corresponding
+        /// <see cref="ExtensionType"/> when the extension is known, or the original field otherwise.
+        /// Fields nested in struct and list types are resolved too.
+        /// Definitions registered with <see cref="ExtensionTypeRegistry.Default"/> take precedence
+        /// over the built-in ones.
+        /// </summary>
+        public static Field Resolve(Field field)
+        {
+            if (field.DataType is ExtensionType)
+            {
+                return field;
+            }
+
+            IArrowType resolvedType = ResolveChildren(field.DataType);
+            if (!ReferenceEquals(resolvedType, field.DataType))
+            {
+                field = new Field(field.Name, resolvedType, field.IsNullable, field.Metadata);
+            }
+
+            if (!field.HasMetadata ||
+                !field.Metadata.TryGetValue(ExtensionNameKey, out string? name))
+            {
+                return field;
+            }
+
+            if (!ExtensionTypeRegistry.Default.TryGetDefinition(name, out ExtensionDefinition definition) &&
+                !s_builtIn.TryGetDefinition(name, out definition))
+            {
+                return field;
+            }
+
+            field.Metadata.TryGetValue(ExtensionMetadataKey, out string? metadata);
+            if (!definition.TryCreateType(field.DataType, metadata ?? string.Empty, out ExtensionType extensionType))
+            {
+                return field;
+            }
+
+            return new Field(field.Name, extensionType, field.IsNullable, field.Metadata);
+        }
+
+        private static IArrowType ResolveChildren(IArrowType type)
+        {
+            if (type is not NestedType nestedType)
+            {
+                return type;
+            }
+
+            Field[] fields = new Field[nestedType.Fields.Count];
+            bool changed = false;
+            for (int i = 0; i < fields.Length; i++)
+            {
+                fields[i] = Resolve(nestedType.Fields[i]);
+                changed |= !ReferenceEquals(fields[i], nestedType.Fields[i]);
+            }
+
+            if (!changed)
+            {
+                return type;
+            }
+
+            switch (type)
+            {
+                case StructType:
+                    return new StructType(fields);
+                case ListType:
+                    return new ListType(fields[0]);
+                case LargeListType:
+                    return new LargeListType(fields[0]);
+                case FixedSizeListType fixedSizeListType:
+                    return new FixedSizeListType(fields[0], fixedSizeListType.ListSize);

Review Comment:
   Follow-up in 5d6b56b89 for the remaining part of this (from the latest review): the resolution handles `large_list` and `fixed_size_list` as well as `list`, but only `list` could be read. Before this PR, a `large_list` or `fixed_size_list` column made the reader throw `NotSupportedException` when it was created. `ValueAt`, `GetValueConverter` and the schema APIs now read both, including when their values are extension arrays (with the same apache/arrow-dotnet#468 workaround as `list`). `NestedGuidFromStorageWithExtensionMetadata` covers `list`, `large_list` and `fixed_size_list` of uuid through `GetValue`, `GetFieldType`, `GetColumnSchema` and `GetSchemaTable`.
   
   The review summary also mentioned null handling for fixed-size binary values. Both conversion paths check for null first, and the `FixedSizeBinary` test now includes a null row that reads as `DBNull`.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(csharp): support extension types in the ADO.NET client [arrow-adbc]

["CurtHagenlocher (via GitHub)" <gi...@apache.org>]

CurtHagenlocher commented on code in PR #4919:
URL: https://github.com/apache/arrow-adbc/pull/4919#discussion_r4307791230


##########
csharp/src/Client/SchemaConverter.cs:
##########
@@ -31,13 +31,13 @@ internal class SchemaConverter
         /// Converts an Arrow <see cref="Schema"/> to a
         /// <see cref="DataTable"/> schema.
         /// </summary>
-        /// <param name="schema">The Arrow schema</param>
+        /// <param name="fields">The fields of the Arrow schema</param>
         /// <param name="adbcStatement">The AdbcStatement to use</param>
         /// <exception cref="ArgumentNullException"></exception>
-        public static DataTable ConvertArrowSchema(Schema schema, AdbcStatement adbcStatement, DecimalBehavior decimalBehavior, StructBehavior structBehavior)
+        public static DataTable ConvertArrowSchema(IReadOnlyList<Field> fields, AdbcStatement adbcStatement, DecimalBehavior decimalBehavior, StructBehavior structBehavior)

Review Comment:
   I don't think this one applies: `SchemaConverter.cs` already has `using System.Collections.Generic;` (line 19), and the client project builds for both `netstandard2.0` and `net8.0`.



##########
csharp/src/Apache.Arrow.Adbc/Extensions/IArrowArrayExtensions.cs:
##########
@@ -156,16 +157,31 @@ public static class IArrowArrayExtensions
                     }
                 case ArrowTypeId.Binary:
                     return ((BinaryArray)arrowArray).GetBytes(index).ToArray();
+                case ArrowTypeId.FixedSizedBinary:
+                    return ((FixedSizeBinaryArray)arrowArray).GetBytes(index).ToArray();
                 case ArrowTypeId.List:
-                    return ((ListArray)arrowArray).GetSlicedValues(index);
+                    return GetListValues((ListArray)arrowArray, index);
                 case ArrowTypeId.Struct:
                     StructArray structArray = (StructArray)arrowArray;
                     return resultType == StructResultType.JsonString ? SerializeToJson(structArray, index) : ParseStructArray(structArray, index);
+                case ArrowTypeId.Extension:
+                    switch (arrowArray)
+                    {
+                        case GuidArray guidArray:
+                            return guidArray.GetGuid(index);
+                        case Bool8Array bool8Array:
+                            return bool8Array.GetValue(index);
+                        case TimestampWithOffsetArray timestampWithOffsetArray:
+                            return timestampWithOffsetArray.GetValue(index);
+                        case ExtensionArray extensionArray:
+                            // Unrecognized extension types are surfaced as their storage values.
+                            return ValueAt(extensionArray.Storage, index, resultType);

Review Comment:
   Fixed in a184940b5. Variant conversion moved from the client into the core `IArrowArrayExtensions` (new `VariantConversions`), so `ValueAt` and `GetValueConverter` handle `VariantArray` everywhere, including inside `ParseStructArray`. A nested variant now becomes the same `Dictionary`/`List`/scalar as a top-level one under Strict, and is serialized as part of the struct's JSON under JsonString. As a result, the `Apache.Arrow.Operations` reference moved from the client to the core library. That adds a 48 KB assembly but no new transitive packages, since core already depends on `Apache.Arrow`/`Scalars` and `System.Text.Json`. The NativeAOT fixture compiles without trim/AOT warnings. `NestedVariant` covers both behaviors.



##########
csharp/src/Client/AdbcCommand.cs:
##########
@@ -298,204 +298,231 @@ private void BindParameters()
                 for (int i = 0; i < fields.Length; i++)
                 {
                     AdbcParameter param = (AdbcParameter)_dbParameterCollection[i];
+                    object? value = param.Value is DBNull ? null : param.Value;
                     switch (param.DbType)
                     {
                         case DbType.Binary:
                             var binaryBuilder = new BinaryArray.Builder();
-                            switch (param.Value)
+                            switch (value)
                             {
                                 case null: binaryBuilder.AppendNull(); break;
                                 case byte[] array: binaryBuilder.Append(array.AsSpan()); break;
-                                default: throw new NotSupportedException($"Values of type {param.Value.GetType().Name} cannot be bound as binary");
+                                default: throw new NotSupportedException($"Values of type {value.GetType().Name} cannot be bound as binary");
                             }
                             parameters[i] = binaryBuilder.Build();
                             break;
                         case DbType.Boolean:
                             var boolBuilder = new BooleanArray.Builder();
-                            switch (param.Value)
+                            switch (value)
                             {
                                 case null: boolBuilder.AppendNull(); break;
                                 case bool boolValue: boolBuilder.Append(boolValue); break;
-                                default: boolBuilder.Append(ConvertValue(param.Value, Convert.ToBoolean, DbType.Boolean)); break;
+                                default: boolBuilder.Append(ConvertValue(value, Convert.ToBoolean, DbType.Boolean)); break;
                             }
                             parameters[i] = boolBuilder.Build();
                             break;
                         case DbType.Byte:
                             var uint8Builder = new UInt8Array.Builder();
-                            switch (param.Value)
+                            switch (value)
                             {
                                 case null: uint8Builder.AppendNull(); break;
                                 case byte byteValue: uint8Builder.Append(byteValue); break;
-                                default: uint8Builder.Append(ConvertValue(param.Value, Convert.ToByte, DbType.Byte)); break;
+                                default: uint8Builder.Append(ConvertValue(value, Convert.ToByte, DbType.Byte)); break;
                             }
                             parameters[i] = uint8Builder.Build();
                             break;
                         case DbType.Date:
                             var dateBuilder = new Date32Array.Builder();
-                            switch (param.Value)
+                            switch (value)
                             {
                                 case null: dateBuilder.AppendNull(); break;
                                 case DateTime datetime: dateBuilder.Append(datetime); break;
 #if NET5_0_OR_GREATER
                                 case DateOnly dateonly: dateBuilder.Append(dateonly); break;
 #endif
-                                default: dateBuilder.Append(ConvertValue(param.Value, Convert.ToDateTime, DbType.Date)); break;
+                                default: dateBuilder.Append(ConvertValue(value, Convert.ToDateTime, DbType.Date)); break;
                             }
                             parameters[i] = dateBuilder.Build();
                             break;
                         case DbType.DateTime:
                             var timestampBuilder = new TimestampArray.Builder();
-                            switch (param.Value)
+                            switch (value)
                             {
                                 case null: timestampBuilder.AppendNull(); break;
                                 case DateTime datetime: timestampBuilder.Append(datetime); break;
-                                default: timestampBuilder.Append(ConvertValue(param.Value, Convert.ToDateTime, DbType.DateTime)); break;
+                                default: timestampBuilder.Append(ConvertValue(value, Convert.ToDateTime, DbType.DateTime)); break;
                             }
                             parameters[i] = timestampBuilder.Build();
                             break;
+                        case DbType.DateTimeOffset:
+                            // Bound as a UTC timestamp, which every driver understands; the
+                            // original offset is not preserved.
+                            var timestampOffsetBuilder = new TimestampArray.Builder(TimeUnit.Microsecond, "UTC");
+                            switch (value)
+                            {
+                                case null: timestampOffsetBuilder.AppendNull(); break;
+                                case DateTimeOffset dateTimeOffset: timestampOffsetBuilder.Append(dateTimeOffset); break;
+                                case DateTime datetime: timestampOffsetBuilder.Append(new DateTimeOffset(datetime)); break;
+                                default: timestampOffsetBuilder.Append(ConvertValue(value, v => DateTimeOffset.Parse(Convert.ToString(v, CultureInfo.InvariantCulture)!, CultureInfo.InvariantCulture), DbType.DateTimeOffset)); break;
+                            }
+                            parameters[i] = timestampOffsetBuilder.Build();
+                            break;
                         case DbType.Decimal:
-                            var value = param.Value switch
+                            var decimalValue = value switch
                             {
                                 null => (SqlDecimal?)null,
                                 SqlDecimal sqlDecimal => sqlDecimal,
                                 decimal d => new SqlDecimal(d),
-                                _ => new SqlDecimal(ConvertValue(param.Value, Convert.ToDecimal, DbType.Decimal)),
+                                _ => new SqlDecimal(ConvertValue(value, Convert.ToDecimal, DbType.Decimal)),
                             };
-                            var decimalBuilder = new Decimal128Array.Builder(new Decimal128Type(value?.Precision ?? 10, value?.Scale ?? 0));
-                            if (value is null)
+                            var decimalBuilder = new Decimal128Array.Builder(new Decimal128Type(decimalValue?.Precision ?? 10, decimalValue?.Scale ?? 0));
+                            if (decimalValue is null)
                             {
                                 decimalBuilder.AppendNull();
                             }
                             else
                             {
-                                decimalBuilder.Append(value.Value);
+                                decimalBuilder.Append(decimalValue.Value);
                             }
                             parameters[i] = decimalBuilder.Build();
                             break;
                         case DbType.Double:
                             var doubleBuilder = new DoubleArray.Builder();
-                            switch (param.Value)
+                            switch (value)
                             {
                                 case null: doubleBuilder.AppendNull(); break;
                                 case double dbl: doubleBuilder.Append(dbl); break;
-                                default: doubleBuilder.Append(ConvertValue(param.Value, Convert.ToDouble, DbType.Double)); break;
+                                default: doubleBuilder.Append(ConvertValue(value, Convert.ToDouble, DbType.Double)); break;
                             }
                             parameters[i] = doubleBuilder.Build();
                             break;
+                        case DbType.Guid:
+                            var guidBuilder = new GuidArray.Builder();
+                            switch (value)
+                            {
+                                case null: guidBuilder.AppendNull(); break;
+                                case Guid guid: guidBuilder.Append(guid); break;
+                                case byte[] bytes when bytes.Length == 16: guidBuilder.Append(new Guid(bytes)); break;

Review Comment:
   Good catch, fixed in a184940b5. The bytes are now read with `GuidArray.RFC4122ToGuid`. `BindGuidAndDateTimeOffsetParameters` now binds `GuidArray.GuidToBytes(guid)` and checks that it comes back as `guid`.



##########
csharp/src/Client/ExtensionTypes.cs:
##########
@@ -0,0 +1,259 @@
+/*
+* 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 Apache.Arrow.Adbc.Extensions;
+using Apache.Arrow.Operations.Shredding;
+using Apache.Arrow.Operations.VariantJson;
+using Apache.Arrow.Scalars.Variant;
+using Apache.Arrow.Types;
+
+namespace Apache.Arrow.Adbc.Client
+{
+    /// <summary>
+    /// Support for Arrow extension types in the ADO.NET client.
+    /// </summary>
+    internal static class ExtensionTypes
+    {
+        internal const string ExtensionNameKey = "ARROW:extension:name";
+        internal const string ExtensionMetadataKey = "ARROW:extension:metadata";
+
+        private static readonly DateTimeOffset s_unixEpoch = new DateTimeOffset(1970, 1, 1, 0, 0, 0, TimeSpan.Zero);
+        private static readonly DateTime s_unixEpochUnspecified = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Unspecified);
+
+        /// <summary>
+        /// The extension types the client understands without any registration by the caller.
+        /// </summary>
+        private static readonly ExtensionTypeRegistry s_builtIn = CreateBuiltInRegistry();
+
+        private static ExtensionTypeRegistry CreateBuiltInRegistry()
+        {
+            ExtensionTypeRegistry registry = new ExtensionTypeRegistry();
+            registry.Register(GuidExtensionDefinition.Instance);
+            registry.Register(Bool8ExtensionDefinition.Instance);
+            registry.Register(TimestampWithOffsetExtensionDefinition.Instance);
+            registry.Register(VariantExtensionDefinition.Instance);
+            return registry;
+        }
+
+        /// <summary>
+        /// Arrays imported through the C Data Interface carry extension types only as field
+        /// metadata over their storage type. Returns a field whose type is the corresponding
+        /// <see cref="ExtensionType"/> when the extension is known, or the original field otherwise.
+        /// Fields nested in struct and list types are resolved too.
+        /// Definitions registered with <see cref="ExtensionTypeRegistry.Default"/> take precedence
+        /// over the built-in ones.
+        /// </summary>
+        public static Field Resolve(Field field)
+        {
+            if (field.DataType is ExtensionType)
+            {
+                return field;
+            }
+
+            IArrowType resolvedType = ResolveChildren(field.DataType);
+            if (!ReferenceEquals(resolvedType, field.DataType))
+            {
+                field = new Field(field.Name, resolvedType, field.IsNullable, field.Metadata);
+            }
+
+            if (!field.HasMetadata ||
+                !field.Metadata.TryGetValue(ExtensionNameKey, out string? name))
+            {
+                return field;
+            }
+
+            if (!ExtensionTypeRegistry.Default.TryGetDefinition(name, out ExtensionDefinition definition) &&
+                !s_builtIn.TryGetDefinition(name, out definition))
+            {
+                return field;
+            }
+
+            field.Metadata.TryGetValue(ExtensionMetadataKey, out string? metadata);
+            if (!definition.TryCreateType(field.DataType, metadata ?? string.Empty, out ExtensionType extensionType))
+            {
+                return field;
+            }
+
+            return new Field(field.Name, extensionType, field.IsNullable, field.Metadata);
+        }
+
+        private static IArrowType ResolveChildren(IArrowType type)
+        {
+            if (type is not NestedType nestedType)
+            {
+                return type;
+            }
+
+            Field[] fields = new Field[nestedType.Fields.Count];
+            bool changed = false;
+            for (int i = 0; i < fields.Length; i++)
+            {
+                fields[i] = Resolve(nestedType.Fields[i]);
+                changed |= !ReferenceEquals(fields[i], nestedType.Fields[i]);
+            }
+
+            if (!changed)
+            {
+                return type;
+            }
+
+            switch (type)
+            {
+                case StructType:
+                    return new StructType(fields);
+                case ListType:
+                    return new ListType(fields[0]);
+                case LargeListType:
+                    return new LargeListType(fields[0]);
+                case FixedSizeListType fixedSizeListType:
+                    return new FixedSizeListType(fields[0], fixedSizeListType.ListSize);

Review Comment:
   Fixed in a184940b5. `GetArrowArrayType` now maps uuid, bool8, timestamp_with_offset and variant element types to their array classes, other extension types to `ExtensionArray`, and `fixed_size_binary` to `FixedSizeBinaryArray`. `NestedGuidFromStorageWithExtensionMetadata` now also checks `GetFieldType`, `GetColumnSchema` and `GetSchemaTable` for the `list<uuid>` column.



##########
csharp/src/Client/SchemaConverter.cs:
##########
@@ -214,6 +215,23 @@ public static Type GetArrowType(Field f, DecimalBehavior decimalBehavior, Struct
                     }
                     goto default;
 
+                case ArrowTypeId.Extension:
+                    switch (f.DataType)
+                    {
+                        case GuidType:
+                            return typeof(Guid);
+                        case Bool8Type:
+                            return typeof(bool);
+                        case TimestampWithOffsetType:
+                            return typeof(DateTimeOffset);
+                        case VariantType:
+                            return structBehavior == StructBehavior.JsonString ? typeof(string) : typeof(object);
+                        default:
+                            // Unrecognized extension types are surfaced as their storage values.
+                            ExtensionType extensionType = (ExtensionType)f.DataType;
+                            return ConvertArrowType(new Field(f.Name, extensionType.StorageType, f.IsNullable), decimalBehavior, structBehavior);

Review Comment:
   Same issue as the comment on `ExtensionTypes.cs`; fixed in a184940b5 as described there.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(csharp): support extension types in the ADO.NET client [arrow-adbc]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #4919:
URL: https://github.com/apache/arrow-adbc/pull/4919#discussion_r4307778240


##########
csharp/src/Client/SchemaConverter.cs:
##########
@@ -31,13 +31,13 @@ internal class SchemaConverter
         /// Converts an Arrow <see cref="Schema"/> to a
         /// <see cref="DataTable"/> schema.
         /// </summary>
-        /// <param name="schema">The Arrow schema</param>
+        /// <param name="fields">The fields of the Arrow schema</param>
         /// <param name="adbcStatement">The AdbcStatement to use</param>
         /// <exception cref="ArgumentNullException"></exception>
-        public static DataTable ConvertArrowSchema(Schema schema, AdbcStatement adbcStatement, DecimalBehavior decimalBehavior, StructBehavior structBehavior)
+        public static DataTable ConvertArrowSchema(IReadOnlyList<Field> fields, AdbcStatement adbcStatement, DecimalBehavior decimalBehavior, StructBehavior structBehavior)

Review Comment:
   This signature introduces `IReadOnlyList<Field>` but the file does not import `System.Collections.Generic` (and the repository disables implicit usings in `csharp/Directory.Build.props:43`). The client project therefore fails to compile with `IReadOnlyList<>` unresolved. Add the using or fully qualify the type.



##########
csharp/src/Apache.Arrow.Adbc/Extensions/IArrowArrayExtensions.cs:
##########
@@ -156,16 +157,31 @@ public static class IArrowArrayExtensions
                     }
                 case ArrowTypeId.Binary:
                     return ((BinaryArray)arrowArray).GetBytes(index).ToArray();
+                case ArrowTypeId.FixedSizedBinary:
+                    return ((FixedSizeBinaryArray)arrowArray).GetBytes(index).ToArray();
                 case ArrowTypeId.List:
-                    return ((ListArray)arrowArray).GetSlicedValues(index);
+                    return GetListValues((ListArray)arrowArray, index);
                 case ArrowTypeId.Struct:
                     StructArray structArray = (StructArray)arrowArray;
                     return resultType == StructResultType.JsonString ? SerializeToJson(structArray, index) : ParseStructArray(structArray, index);
+                case ArrowTypeId.Extension:
+                    switch (arrowArray)
+                    {
+                        case GuidArray guidArray:
+                            return guidArray.GetGuid(index);
+                        case Bool8Array bool8Array:
+                            return bool8Array.GetValue(index);
+                        case TimestampWithOffsetArray timestampWithOffsetArray:
+                            return timestampWithOffsetArray.GetValue(index);
+                        case ExtensionArray extensionArray:
+                            // Unrecognized extension types are surfaced as their storage values.
+                            return ValueAt(extensionArray.Storage, index, resultType);

Review Comment:
   This fallback is also used when ParseStructArray walks a struct containing a resolved arrow.parquet.variant child. Unlike a top-level variant, that child is reduced to its storage array here, so StructBehavior.JsonString/Strict returns the variant's storage representation instead of VariantJsonWriter output or the logical Dictionary/List/scalar value. Handle VariantArray in the recursive conversion path as well.



##########
csharp/src/Client/AdbcCommand.cs:
##########
@@ -298,204 +298,231 @@ private void BindParameters()
                 for (int i = 0; i < fields.Length; i++)
                 {
                     AdbcParameter param = (AdbcParameter)_dbParameterCollection[i];
+                    object? value = param.Value is DBNull ? null : param.Value;
                     switch (param.DbType)
                     {
                         case DbType.Binary:
                             var binaryBuilder = new BinaryArray.Builder();
-                            switch (param.Value)
+                            switch (value)
                             {
                                 case null: binaryBuilder.AppendNull(); break;
                                 case byte[] array: binaryBuilder.Append(array.AsSpan()); break;
-                                default: throw new NotSupportedException($"Values of type {param.Value.GetType().Name} cannot be bound as binary");
+                                default: throw new NotSupportedException($"Values of type {value.GetType().Name} cannot be bound as binary");
                             }
                             parameters[i] = binaryBuilder.Build();
                             break;
                         case DbType.Boolean:
                             var boolBuilder = new BooleanArray.Builder();
-                            switch (param.Value)
+                            switch (value)
                             {
                                 case null: boolBuilder.AppendNull(); break;
                                 case bool boolValue: boolBuilder.Append(boolValue); break;
-                                default: boolBuilder.Append(ConvertValue(param.Value, Convert.ToBoolean, DbType.Boolean)); break;
+                                default: boolBuilder.Append(ConvertValue(value, Convert.ToBoolean, DbType.Boolean)); break;
                             }
                             parameters[i] = boolBuilder.Build();
                             break;
                         case DbType.Byte:
                             var uint8Builder = new UInt8Array.Builder();
-                            switch (param.Value)
+                            switch (value)
                             {
                                 case null: uint8Builder.AppendNull(); break;
                                 case byte byteValue: uint8Builder.Append(byteValue); break;
-                                default: uint8Builder.Append(ConvertValue(param.Value, Convert.ToByte, DbType.Byte)); break;
+                                default: uint8Builder.Append(ConvertValue(value, Convert.ToByte, DbType.Byte)); break;
                             }
                             parameters[i] = uint8Builder.Build();
                             break;
                         case DbType.Date:
                             var dateBuilder = new Date32Array.Builder();
-                            switch (param.Value)
+                            switch (value)
                             {
                                 case null: dateBuilder.AppendNull(); break;
                                 case DateTime datetime: dateBuilder.Append(datetime); break;
 #if NET5_0_OR_GREATER
                                 case DateOnly dateonly: dateBuilder.Append(dateonly); break;
 #endif
-                                default: dateBuilder.Append(ConvertValue(param.Value, Convert.ToDateTime, DbType.Date)); break;
+                                default: dateBuilder.Append(ConvertValue(value, Convert.ToDateTime, DbType.Date)); break;
                             }
                             parameters[i] = dateBuilder.Build();
                             break;
                         case DbType.DateTime:
                             var timestampBuilder = new TimestampArray.Builder();
-                            switch (param.Value)
+                            switch (value)
                             {
                                 case null: timestampBuilder.AppendNull(); break;
                                 case DateTime datetime: timestampBuilder.Append(datetime); break;
-                                default: timestampBuilder.Append(ConvertValue(param.Value, Convert.ToDateTime, DbType.DateTime)); break;
+                                default: timestampBuilder.Append(ConvertValue(value, Convert.ToDateTime, DbType.DateTime)); break;
                             }
                             parameters[i] = timestampBuilder.Build();
                             break;
+                        case DbType.DateTimeOffset:
+                            // Bound as a UTC timestamp, which every driver understands; the
+                            // original offset is not preserved.
+                            var timestampOffsetBuilder = new TimestampArray.Builder(TimeUnit.Microsecond, "UTC");
+                            switch (value)
+                            {
+                                case null: timestampOffsetBuilder.AppendNull(); break;
+                                case DateTimeOffset dateTimeOffset: timestampOffsetBuilder.Append(dateTimeOffset); break;
+                                case DateTime datetime: timestampOffsetBuilder.Append(new DateTimeOffset(datetime)); break;
+                                default: timestampOffsetBuilder.Append(ConvertValue(value, v => DateTimeOffset.Parse(Convert.ToString(v, CultureInfo.InvariantCulture)!, CultureInfo.InvariantCulture), DbType.DateTimeOffset)); break;
+                            }
+                            parameters[i] = timestampOffsetBuilder.Build();
+                            break;
                         case DbType.Decimal:
-                            var value = param.Value switch
+                            var decimalValue = value switch
                             {
                                 null => (SqlDecimal?)null,
                                 SqlDecimal sqlDecimal => sqlDecimal,
                                 decimal d => new SqlDecimal(d),
-                                _ => new SqlDecimal(ConvertValue(param.Value, Convert.ToDecimal, DbType.Decimal)),
+                                _ => new SqlDecimal(ConvertValue(value, Convert.ToDecimal, DbType.Decimal)),
                             };
-                            var decimalBuilder = new Decimal128Array.Builder(new Decimal128Type(value?.Precision ?? 10, value?.Scale ?? 0));
-                            if (value is null)
+                            var decimalBuilder = new Decimal128Array.Builder(new Decimal128Type(decimalValue?.Precision ?? 10, decimalValue?.Scale ?? 0));
+                            if (decimalValue is null)
                             {
                                 decimalBuilder.AppendNull();
                             }
                             else
                             {
-                                decimalBuilder.Append(value.Value);
+                                decimalBuilder.Append(decimalValue.Value);
                             }
                             parameters[i] = decimalBuilder.Build();
                             break;
                         case DbType.Double:
                             var doubleBuilder = new DoubleArray.Builder();
-                            switch (param.Value)
+                            switch (value)
                             {
                                 case null: doubleBuilder.AppendNull(); break;
                                 case double dbl: doubleBuilder.Append(dbl); break;
-                                default: doubleBuilder.Append(ConvertValue(param.Value, Convert.ToDouble, DbType.Double)); break;
+                                default: doubleBuilder.Append(ConvertValue(value, Convert.ToDouble, DbType.Double)); break;
                             }
                             parameters[i] = doubleBuilder.Build();
                             break;
+                        case DbType.Guid:
+                            var guidBuilder = new GuidArray.Builder();
+                            switch (value)
+                            {
+                                case null: guidBuilder.AppendNull(); break;
+                                case Guid guid: guidBuilder.Append(guid); break;
+                                case byte[] bytes when bytes.Length == 16: guidBuilder.Append(new Guid(bytes)); break;

Review Comment:
   `GuidArray` stores UUID bytes in RFC 4122 big-endian order, but `new Guid(bytes)` interprets the first three fields using .NET's legacy little-endian layout. A 16-byte canonical `arrow.uuid` parameter will therefore be byte-swapped before binding (for example, the bytes returned by `GuidArray.GuidToBytes(guid)` will not bind back to `guid`). Convert canonical bytes with `GuidArray.RFC4122ToGuid(bytes)` instead.



##########
csharp/src/Client/SchemaConverter.cs:
##########
@@ -214,6 +215,23 @@ public static Type GetArrowType(Field f, DecimalBehavior decimalBehavior, Struct
                     }
                     goto default;
 
+                case ArrowTypeId.Extension:
+                    switch (f.DataType)
+                    {
+                        case GuidType:
+                            return typeof(Guid);
+                        case Bool8Type:
+                            return typeof(bool);
+                        case TimestampWithOffsetType:
+                            return typeof(DateTimeOffset);
+                        case VariantType:
+                            return structBehavior == StructBehavior.JsonString ? typeof(string) : typeof(object);
+                        default:
+                            // Unrecognized extension types are surfaced as their storage values.
+                            ExtensionType extensionType = (ExtensionType)f.DataType;
+                            return ConvertArrowType(new Field(f.Name, extensionType.StorageType, f.IsNullable), decimalBehavior, structBehavior);

Review Comment:
   The new nested extension resolution can produce a ListType whose value type is GuidType, Bool8Type, TimestampWithOffsetType, or VariantType, but ConvertArrowType still routes lists through GetArrowArrayType(valueType), whose switch has no extension cases. Calling GetFieldType, GetColumnSchema, or GetSchemaTable for a list of one of these extensions therefore throws InvalidCastException; update list/array-type conversion to handle resolved extension children.



##########
csharp/src/Client/ExtensionTypes.cs:
##########
@@ -0,0 +1,259 @@
+/*
+* 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 Apache.Arrow.Adbc.Extensions;
+using Apache.Arrow.Operations.Shredding;
+using Apache.Arrow.Operations.VariantJson;
+using Apache.Arrow.Scalars.Variant;
+using Apache.Arrow.Types;
+
+namespace Apache.Arrow.Adbc.Client
+{
+    /// <summary>
+    /// Support for Arrow extension types in the ADO.NET client.
+    /// </summary>
+    internal static class ExtensionTypes
+    {
+        internal const string ExtensionNameKey = "ARROW:extension:name";
+        internal const string ExtensionMetadataKey = "ARROW:extension:metadata";
+
+        private static readonly DateTimeOffset s_unixEpoch = new DateTimeOffset(1970, 1, 1, 0, 0, 0, TimeSpan.Zero);
+        private static readonly DateTime s_unixEpochUnspecified = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Unspecified);
+
+        /// <summary>
+        /// The extension types the client understands without any registration by the caller.
+        /// </summary>
+        private static readonly ExtensionTypeRegistry s_builtIn = CreateBuiltInRegistry();
+
+        private static ExtensionTypeRegistry CreateBuiltInRegistry()
+        {
+            ExtensionTypeRegistry registry = new ExtensionTypeRegistry();
+            registry.Register(GuidExtensionDefinition.Instance);
+            registry.Register(Bool8ExtensionDefinition.Instance);
+            registry.Register(TimestampWithOffsetExtensionDefinition.Instance);
+            registry.Register(VariantExtensionDefinition.Instance);
+            return registry;
+        }
+
+        /// <summary>
+        /// Arrays imported through the C Data Interface carry extension types only as field
+        /// metadata over their storage type. Returns a field whose type is the corresponding
+        /// <see cref="ExtensionType"/> when the extension is known, or the original field otherwise.
+        /// Fields nested in struct and list types are resolved too.
+        /// Definitions registered with <see cref="ExtensionTypeRegistry.Default"/> take precedence
+        /// over the built-in ones.
+        /// </summary>
+        public static Field Resolve(Field field)
+        {
+            if (field.DataType is ExtensionType)
+            {
+                return field;
+            }
+
+            IArrowType resolvedType = ResolveChildren(field.DataType);
+            if (!ReferenceEquals(resolvedType, field.DataType))
+            {
+                field = new Field(field.Name, resolvedType, field.IsNullable, field.Metadata);
+            }
+
+            if (!field.HasMetadata ||
+                !field.Metadata.TryGetValue(ExtensionNameKey, out string? name))
+            {
+                return field;
+            }
+
+            if (!ExtensionTypeRegistry.Default.TryGetDefinition(name, out ExtensionDefinition definition) &&
+                !s_builtIn.TryGetDefinition(name, out definition))
+            {
+                return field;
+            }
+
+            field.Metadata.TryGetValue(ExtensionMetadataKey, out string? metadata);
+            if (!definition.TryCreateType(field.DataType, metadata ?? string.Empty, out ExtensionType extensionType))
+            {
+                return field;
+            }
+
+            return new Field(field.Name, extensionType, field.IsNullable, field.Metadata);
+        }
+
+        private static IArrowType ResolveChildren(IArrowType type)
+        {
+            if (type is not NestedType nestedType)
+            {
+                return type;
+            }
+
+            Field[] fields = new Field[nestedType.Fields.Count];
+            bool changed = false;
+            for (int i = 0; i < fields.Length; i++)
+            {
+                fields[i] = Resolve(nestedType.Fields[i]);
+                changed |= !ReferenceEquals(fields[i], nestedType.Fields[i]);
+            }
+
+            if (!changed)
+            {
+                return type;
+            }
+
+            switch (type)
+            {
+                case StructType:
+                    return new StructType(fields);
+                case ListType:
+                    return new ListType(fields[0]);
+                case LargeListType:
+                    return new LargeListType(fields[0]);
+                case FixedSizeListType fixedSizeListType:
+                    return new FixedSizeListType(fields[0], fixedSizeListType.ListSize);

Review Comment:
   Resolving nested UUID/bool8/other extensions creates types such as `ListType(GuidType)`, but `SchemaConverter.ConvertArrowType` sends every list value type to `GetArrowArrayType`, which has no extension cases. Consequently a reader can read the list value but `GetFieldType`, `GetColumnSchema`, or `GetSchemaTable` throws for `list<uuid>` (and the other extension element types). Add schema mappings for extension element arrays (and their storage fallback) and cover the schema APIs in the nested-extension test.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(csharp): support extension types in the ADO.NET client [arrow-adbc]

["CurtHagenlocher (via GitHub)" <gi...@apache.org>]

CurtHagenlocher commented on PR #4919:
URL: https://github.com/apache/arrow-adbc/pull/4919#issuecomment-6253168847

   Re the review note about NaN and Infinity in variant JSON: variants use the same JSON writer as structs, and on `main` that writer already throws for non-finite float and double struct fields. Arrow's own `VariantJsonWriter` also rejects them. Since this predates this PR, it's fixed separately in #4921, which writes them as `"NaN"`, `"Infinity"` and `"-Infinity"`. Variants will get the same behavior through the shared writer; once #4921 merges, I'll add a variant regression test here.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(csharp): support extension types in the ADO.NET client [arrow-adbc]

["CurtHagenlocher (via GitHub)" <gi...@apache.org>]

CurtHagenlocher commented on code in PR #4919:
URL: https://github.com/apache/arrow-adbc/pull/4919#discussion_r4307757633


##########
csharp/src/Client/AdbcDataReader.cs:
##########
@@ -244,6 +257,27 @@ public override string GetName(int ordinal)
             return this.schema.GetFieldByIndex(ordinal)?.Name ?? string.Empty;
         }
 
+        /// <summary>
+        /// Gets the Arrow array for a column in the current record batch. Columns with an
+        /// extension type known only from field metadata are wrapped in the extension array.
+        /// </summary>
+        /// <param name="ordinal">The zero-based column ordinal.</param>
+        public IArrowArray GetArrowArray(int ordinal)
+        {
+            IArrowArray? column = this.columns[ordinal];
+            if (column == null)
+            {
+                column = this.RecordBatch.Column(ordinal);

Review Comment:
   Fixed in fc097d76f. `GetArrowArray` now checks `RecordBatch` (which throws once the reader is closed) before consulting the cache, and `Dispose` clears the cache. `ClosedReaderDoesNotReturnCachedColumns` covers read → access → close → access for both `GetValue` and `GetArrowArray`.



##########
csharp/src/Client/AdbcCommand.cs:
##########
@@ -382,6 +395,17 @@ private void BindParameters()
                             }
                             parameters[i] = doubleBuilder.Build();
                             break;
+                        case DbType.Guid:
+                            var guidBuilder = new GuidArray.Builder();
+                            switch (param.Value)
+                            {
+                                case null: guidBuilder.AppendNull(); break;

Review Comment:
   Fixed in fc097d76f. No `DbType` branch handled `DBNull.Value` before this PR either, so rather than special-casing the two new branches, `BindParameters` now treats `DBNull.Value` as null for every type. The field's nullability uses the normalized value too. `BindDBNullParameter` covers Guid, DateTimeOffset, Int32 and String.



##########
csharp/src/Client/ExtensionTypes.cs:
##########
@@ -0,0 +1,186 @@
+/*
+* 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 Apache.Arrow.Adbc.Extensions;
+using Apache.Arrow.Operations.Shredding;
+using Apache.Arrow.Operations.VariantJson;
+using Apache.Arrow.Scalars.Variant;
+using Apache.Arrow.Types;
+
+namespace Apache.Arrow.Adbc.Client
+{
+    /// <summary>
+    /// Support for Arrow extension types in the ADO.NET client.
+    /// </summary>
+    internal static class ExtensionTypes
+    {
+        internal const string ExtensionNameKey = "ARROW:extension:name";
+        internal const string ExtensionMetadataKey = "ARROW:extension:metadata";
+
+        private static readonly DateTimeOffset s_unixEpoch = new DateTimeOffset(1970, 1, 1, 0, 0, 0, TimeSpan.Zero);
+
+        /// <summary>
+        /// The extension types the client understands without any registration by the caller.
+        /// </summary>
+        private static readonly ExtensionTypeRegistry s_builtIn = CreateBuiltInRegistry();
+
+        private static ExtensionTypeRegistry CreateBuiltInRegistry()
+        {
+            ExtensionTypeRegistry registry = new ExtensionTypeRegistry();
+            registry.Register(GuidExtensionDefinition.Instance);
+            registry.Register(Bool8ExtensionDefinition.Instance);
+            registry.Register(TimestampWithOffsetExtensionDefinition.Instance);
+            registry.Register(VariantExtensionDefinition.Instance);
+            return registry;
+        }
+
+        /// <summary>
+        /// Arrays imported through the C Data Interface carry extension types only as field
+        /// metadata over their storage type. Returns a field whose type is the corresponding
+        /// <see cref="ExtensionType"/> when the extension is known, or the original field otherwise.
+        /// Definitions registered with <see cref="ExtensionTypeRegistry.Default"/> take precedence
+        /// over the built-in ones.
+        /// </summary>
+        public static Field Resolve(Field field)
+        {
+            if (field.DataType is ExtensionType ||
+                !field.HasMetadata ||
+                !field.Metadata.TryGetValue(ExtensionNameKey, out string? name))
+            {
+                return field;

Review Comment:
   Fixed in fc097d76f. `ExtensionTypes.Resolve` now resolves fields nested in struct, list, large list and fixed-size list types. When a column's type changes, the reader rebuilds its `ArrayData` tree with the resolved types and lets `ArrowArrayFactory` create the extension arrays at every level, sharing the original buffers. `NestedGuidFromStorageWithExtensionMetadata` covers a uuid child in a struct (Strict and JsonString) and in a list.
   
   The list case also exposed apache/arrow-dotnet#468: `ListArray.GetSlicedValues` returns null when the values are an `ExtensionArray`. The core list converter works around it by slicing directly.



##########
csharp/src/Client/ExtensionTypes.cs:
##########
@@ -0,0 +1,186 @@
+/*
+* 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 Apache.Arrow.Adbc.Extensions;
+using Apache.Arrow.Operations.Shredding;
+using Apache.Arrow.Operations.VariantJson;
+using Apache.Arrow.Scalars.Variant;
+using Apache.Arrow.Types;
+
+namespace Apache.Arrow.Adbc.Client
+{
+    /// <summary>
+    /// Support for Arrow extension types in the ADO.NET client.
+    /// </summary>
+    internal static class ExtensionTypes
+    {
+        internal const string ExtensionNameKey = "ARROW:extension:name";
+        internal const string ExtensionMetadataKey = "ARROW:extension:metadata";
+
+        private static readonly DateTimeOffset s_unixEpoch = new DateTimeOffset(1970, 1, 1, 0, 0, 0, TimeSpan.Zero);
+
+        /// <summary>
+        /// The extension types the client understands without any registration by the caller.
+        /// </summary>
+        private static readonly ExtensionTypeRegistry s_builtIn = CreateBuiltInRegistry();
+
+        private static ExtensionTypeRegistry CreateBuiltInRegistry()
+        {
+            ExtensionTypeRegistry registry = new ExtensionTypeRegistry();
+            registry.Register(GuidExtensionDefinition.Instance);
+            registry.Register(Bool8ExtensionDefinition.Instance);
+            registry.Register(TimestampWithOffsetExtensionDefinition.Instance);
+            registry.Register(VariantExtensionDefinition.Instance);
+            return registry;
+        }
+
+        /// <summary>
+        /// Arrays imported through the C Data Interface carry extension types only as field
+        /// metadata over their storage type. Returns a field whose type is the corresponding
+        /// <see cref="ExtensionType"/> when the extension is known, or the original field otherwise.
+        /// Definitions registered with <see cref="ExtensionTypeRegistry.Default"/> take precedence
+        /// over the built-in ones.
+        /// </summary>
+        public static Field Resolve(Field field)
+        {
+            if (field.DataType is ExtensionType ||
+                !field.HasMetadata ||
+                !field.Metadata.TryGetValue(ExtensionNameKey, out string? name))
+            {
+                return field;
+            }
+
+            if (!ExtensionTypeRegistry.Default.TryGetDefinition(name, out ExtensionDefinition definition) &&
+                !s_builtIn.TryGetDefinition(name, out definition))
+            {
+                return field;
+            }
+
+            field.Metadata.TryGetValue(ExtensionMetadataKey, out string? metadata);
+            if (!definition.TryCreateType(field.DataType, metadata ?? string.Empty, out ExtensionType extensionType))
+            {
+                return field;
+            }
+
+            return new Field(field.Name, extensionType, field.IsNullable, field.Metadata);
+        }
+
+        /// <summary>
+        /// Gets a value converter for the given type, adding the client's handling of
+        /// variants to the conversions supplied by <see cref="IArrowArrayExtensions"/>.
+        /// </summary>
+        public static Func<IArrowArray, int, object?> GetValueConverter(IArrowType type, StructBehavior structBehavior)
+        {
+            if (type is VariantType)
+            {
+                if (structBehavior == StructBehavior.JsonString)
+                {
+                    return (array, index) => array.IsNull(index) ? null : VariantJsonWriter.ToJson(GetVariantValue((VariantArray)array, index));
+                }
+                return (array, index) => array.IsNull(index) ? null : ToObject(GetVariantValue((VariantArray)array, index));
+            }
+
+            StructResultType structResultType = structBehavior == StructBehavior.JsonString ? StructResultType.JsonString : StructResultType.Object;
+            return type.GetValueConverter(structResultType);
+        }
+
+        private static VariantValue GetVariantValue(VariantArray array, int index)
+        {
+            return array.IsShredded ? array.GetLogicalVariantValue(index) : array.GetVariantValue(index);
+        }
+
+        /// <summary>
+        /// Converts a variant value to the equivalent .NET value. Objects become
+        /// <see cref="Dictionary{TKey, TValue}"/> and arrays become <see cref="List{T}"/>.
+        /// </summary>
+        internal static object? ToObject(VariantValue value)
+        {
+            if (value.IsObject)
+            {
+                Dictionary<string, object?> result = new Dictionary<string, object?>();
+                foreach (KeyValuePair<string, VariantValue> field in value.AsObject())
+                {
+                    result.Add(field.Key, ToObject(field.Value));
+                }
+                return result;
+            }
+
+            if (value.IsArray)
+            {
+                IReadOnlyList<VariantValue> elements = value.AsArray();
+                List<object?> result = new List<object?>(elements.Count);
+                foreach (VariantValue element in elements)
+                {
+                    result.Add(ToObject(element));
+                }
+                return result;
+            }
+
+            switch (value.PrimitiveType)
+            {
+                case VariantPrimitiveType.NullType:
+                    return null;
+                case VariantPrimitiveType.BooleanTrue:
+                case VariantPrimitiveType.BooleanFalse:
+                    return value.AsBoolean();
+                case VariantPrimitiveType.Int8:
+                    return value.AsInt8();
+                case VariantPrimitiveType.Int16:
+                    return value.AsInt16();
+                case VariantPrimitiveType.Int32:
+                    return value.AsInt32();
+                case VariantPrimitiveType.Int64:
+                    return value.AsInt64();
+                case VariantPrimitiveType.Float:
+                    return value.AsFloat();
+                case VariantPrimitiveType.Double:
+                    return value.AsDouble();
+                case VariantPrimitiveType.Decimal4:
+                case VariantPrimitiveType.Decimal8:
+                    return value.AsDecimal();
+                case VariantPrimitiveType.Decimal16:
+                    return value.AsSqlDecimal();
+                case VariantPrimitiveType.Date:
+                    return value.AsDate();
+                case VariantPrimitiveType.Timestamp:
+                    return value.AsTimestamp();
+                case VariantPrimitiveType.TimestampNtz:
+                    return value.AsTimestampNtz();
+                case VariantPrimitiveType.TimestampTzNanos:
+                    return s_unixEpoch.AddTicks(value.AsTimestampTzNanos() / 100);
+                case VariantPrimitiveType.TimestampNtzNanos:
+                    return s_unixEpoch.UtcDateTime.AddTicks(value.AsTimestampNtzNanos() / 100);

Review Comment:
   Fixed in fc097d76f. It now uses an epoch with `DateTimeKind.Unspecified`, matching Arrow's own `VariantValue.AsTimestampNtz`. `VariantTimestampNtzIsUnspecified` checks the returned `Kind`.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(csharp): support extension types in the ADO.NET client [arrow-adbc]

["CurtHagenlocher (via GitHub)" <gi...@apache.org>]

CurtHagenlocher commented on code in PR #4919:
URL: https://github.com/apache/arrow-adbc/pull/4919#discussion_r4307848586


##########
csharp/src/Apache.Arrow.Adbc/Extensions/IArrowArrayExtensions.cs:
##########
@@ -156,16 +157,31 @@ public static class IArrowArrayExtensions
                     }
                 case ArrowTypeId.Binary:
                     return ((BinaryArray)arrowArray).GetBytes(index).ToArray();
+                case ArrowTypeId.FixedSizedBinary:
+                    return ((FixedSizeBinaryArray)arrowArray).GetBytes(index).ToArray();
                 case ArrowTypeId.List:
-                    return ((ListArray)arrowArray).GetSlicedValues(index);
+                    return GetListValues((ListArray)arrowArray, index);
                 case ArrowTypeId.Struct:
                     StructArray structArray = (StructArray)arrowArray;
                     return resultType == StructResultType.JsonString ? SerializeToJson(structArray, index) : ParseStructArray(structArray, index);
+                case ArrowTypeId.Extension:
+                    switch (arrowArray)
+                    {
+                        case GuidArray guidArray:
+                            return guidArray.GetGuid(index);
+                        case Bool8Array bool8Array:
+                            return bool8Array.GetValue(index);
+                        case TimestampWithOffsetArray timestampWithOffsetArray:
+                            return timestampWithOffsetArray.GetValue(index);
+                        case ExtensionArray extensionArray:
+                            // Unrecognized extension types are surfaced as their storage values.
+                            return ValueAt(extensionArray.Storage, index, resultType);

Review Comment:
   Follow-up in ac532fed4: the `Apache.Arrow.Operations` dependency is gone again. Unshredded variants are decoded with `Apache.Arrow` alone and formatted with the same JSON writer as structs, and nested variants still convert as described above. Shredded variants are now opt-in: an application references `Apache.Arrow.Operations` and sets `ShreddedVariants.Reader = (array, index) => array.GetLogicalVariantValue(index)`. Without that, reading a shredded variant throws a `NotSupportedException` that explains how to enable it.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org