cmettler commented on code in PR #284: URL: https://github.com/apache/arrow-dotnet/pull/284#discussion_r3539384861
########## src/Apache.Arrow.Serialization.Generator/Models.cs: ########## @@ -0,0 +1,151 @@ +// 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.Collections.Generic; + +#nullable enable + +namespace Apache.Arrow.Serialization.Generator +{ + internal enum TypeKind2 Review Comment: All open issues should be addressed. # Variant integration — separate PR or fold in here? Since Variant landed in Apache.Arrow, I've prototyped a full serializer support for it on a local branch stacked on this one (~9 commits, 231 tests on net8.0 / 216 on net462. Before pushing anything: **would you prefer this as a follow-up PR once this one merges, or folded into this PR?** My instinct is follow-up — this PR is large enough — but happy either way. A quick tour, in three layers: **1. `VariantValue` properties map to the Variant extension type** — a column that holds any shape per row: ```csharp using Apache.Arrow.Scalars.Variant; [ArrowSerializable] public partial record Event { public string Name { get; init; } = ""; public VariantValue Payload { get; init; } } var evt = new Event { Name = "click", Payload = VariantValue.FromObject(new Dictionary<string, VariantValue> { ["x"] = VariantValue.FromInt32(10), ["tags"] = VariantValue.FromArray(VariantValue.FromString("a"), VariantValue.FromString("b")), }), }; var restored = Event.FromRecordBatch(Event.ToRecordBatch(evt)); int x = restored.Payload.AsObject()["x"].AsInt32(); // IPC round-trips too ``` **2. `[ArrowType("variant")]` stores any POCO/collection property as a self-describing Variant column** instead of Struct/List — fully source-generated, recursive, no reflection: ```csharp [ArrowSerializable] public partial record Order { public Address Shipping { get; init; } = new(); // Struct column (default) [ArrowType("variant")] public Address? Billing { get; init; } // Variant column [ArrowType("variant")] public Dictionary<string, double> Prices { get; init; } = new(); } ``` On read, missing fields default and unknown fields are ignored — old readers survive new writers and vice versa. **3. An alternative wire format for `[ArrowPolymorphic]`** — instead of the flat discriminator + union-of-all-fields schema, one Variant column with self-describing rows: ```csharp [ArrowPolymorphic(Format = ArrowPolymorphicFormat.Variant)] [ArrowDerivedType(typeof(Order), "order")] [ArrowDerivedType(typeof(Ping), "ping")] public abstract partial record Message; // wire: // {"$type":"order","Id":"A1","Shipping":{"City":"Berlin"},"Tags":["a","b"]} // {"$type":"ping","Seq":7} ``` The schema never widens as derived types are added, and the flat union's property-type restrictions disappear (nested records, collections, and dictionaries all work). Flat union stays the default — it's the right choice for columnar analytics; the README has a trade-off table. There are also opt-in read policies (`MissingFieldHandling` / `TypeMismatchHandling`) for consuming foreign-written batches. **One piece you may want in *this* PR regardless:** while testing IPC round-trips I found that extension columns don't rehydrate after `ArrowStreamReader` — `ExtensionTypeRegistry.Default` starts empty, so the generated `(GuidArray)` cast for the existing uuid mapping fails on any deserialized stream. The variant branch fixes this by registering the uuid (and variant) definitions from a module initializer (never overwriting existing registrations), plus a Guid IPC regression test. Happy to cherry-pick just that fix into this PR now if you'd like. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
