feat(csharp): make the driver AOT/trim-safe under net10.0#509
feat(csharp): make the driver AOT/trim-safe under net10.0#509CurtHagenlocher wants to merge 5 commits into
Conversation
Add net10.0 to the driver's target frameworks and enable the trim, single-file, and NativeAOT Roslyn analyzers for it (IsAotCompatible). A normal `dotnet build` now surfaces IL2xxx/IL3xxx warnings — which, with TreatWarningsAsErrors, fail the build — so the driver is verified AOT-ready without yet producing a native build. Enabling the analyzers flagged one category of problem: reflection-based System.Text.Json. Fixes: - Route the concrete-DTO (de)serialization through source-generated JsonSerializerContexts: SEA request/response models, the DESC-EXTENDED result, the feature-flags response, the config dictionary, and the telemetry request envelope. Options mirror the prior runtime JsonSerializerOptions; unused options fields removed. - ComplexTypeSerializingStream serializes polymorphic Arrow value graphs (ARRAY/MAP/STRUCT), which source-gen can't model. Replace JsonSerializer.Serialize(object) with a recursive Utf8JsonWriter walk over the closed set of Arrow scalar/list/dict types — same output, no reflection. - TelemetryFrontendLog nests a protobuf type whose graph trips the source generator (SYSLIB1031: nested `Type` enum name collisions), even with a property-level [JsonConverter]. Serialize its small, stable envelope by hand (TelemetryPayloadWriter), reusing the proto converter for the leaf. The proto-free TelemetryRequest stays on source-gen; the reflection-based TelemetryJsonOptions is kept for the test suite. All four TFMs (netstandard2.0;net472;net8.0;net10.0) build clean with no IL/SYSLIB warnings; net10.0 goes from 28 IL warnings to 0. The 825 unit tests pass, including the telemetry tests that exercise the production serialization path.
There was a problem hiding this comment.
Pull request overview
This PR adds a net10.0 target for the Databricks C# driver and makes key JSON serialization paths trim/NativeAOT-safe by switching from reflection-based System.Text.Json usage to source-generated JsonSerializerContexts and targeted manual Utf8JsonWriter serialization where source-gen cannot be used.
Changes:
- Add
net10.0TFM and enableIsAotCompatible/analyzers undernet10.0to surface AOT/trimming warnings during normal builds. - Replace reflection-based JSON (de)serialization with source-generated
JsonSerializerContexts for SEA models, DESC-EXTENDED results, feature flags, and config dictionary parsing. - Remove reflection-based serialization in two special cases via manual writers: telemetry frontend-log envelope serialization and complex Arrow value graph serialization.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| csharp/src/Telemetry/TelemetryPayloadWriter.cs | Adds manual, AOT-safe frontend-log JSON writer that delegates proto leaf formatting to the existing proto converter. |
| csharp/src/Telemetry/TelemetryJsonContext.cs | Adds source-generated STJ context for TelemetryRequest to avoid reflection during request serialization. |
| csharp/src/Telemetry/ProtoJsonConverter.cs | Exposes a reusable WriteValue method so manual telemetry serialization can share the exact proto JSON formatting. |
| csharp/src/Telemetry/DatabricksTelemetryExporter.cs | Routes telemetry request/log serialization through the new AOT-safe serializer/context. |
| csharp/src/StatementExecution/StatementExecutionStatement.cs | Switches DESC-EXTENDED JSON parsing to source-generated type metadata. |
| csharp/src/StatementExecution/StatementExecutionJsonContext.cs | Adds source-generated STJ context for SEA request/response model types. |
| csharp/src/StatementExecution/StatementExecutionClient.cs | Replaces runtime JsonSerializerOptions usage with source-generated contexts for SEA (de)serialization. |
| csharp/src/Result/DescTableJsonContext.cs | Adds source-generated STJ context for DescTableExtendedResult. |
| csharp/src/FeatureFlagsJsonContext.cs | Adds source-generated STJ context for FeatureFlagsResponse. |
| csharp/src/FeatureFlagContext.cs | Uses source-generated metadata to deserialize feature flags without reflection. |
| csharp/src/DatabricksStatement.cs | Switches DESC-EXTENDED JSON parsing to source-generated type metadata. |
| csharp/src/DatabricksConfiguration.cs | Deserializes free-form config JSON via a source-generated dictionary context (AOT/trim-safe). |
| csharp/src/DatabricksConfigJsonContext.cs | Adds source-generated STJ context + options mirroring prior runtime options for config deserialization. |
| csharp/src/ComplexTypeSerializingStream.cs | Replaces JsonSerializer.Serialize(object) with a manual Utf8JsonWriter walk over Arrow scalar/list/dict graphs (AOT/trim-safe). |
| csharp/src/AdbcDrivers.Databricks.csproj | Adds net10.0 and enables IsAotCompatible + analyzers for that target. |
| csharp/Directory.Packages.props | Enables transitive central pinning and adds version pins needed for the new build configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks @CurtHagenlocher , may I know what is the reason for this PR? My only concern is since it does not use reflection anymore, any new fields will need an explicitly map. |
The only reason to enable For now, this is an option we're considering rather than something we've decided to definitely do.
I believe that particular serializer is for converting structured Arrow data into JSON. We can probably gain confidence and efficiency by going straight from Arrow into the Json writer instead of using .NET structures as an intermediate. I'll look into making that change. |
…OT serializers Follow-up to the net10.0 AOT work (26c483f). - Rewrite ComplexTypeSerializingStream to emit ARRAY/MAP/STRUCT values directly from the Arrow arrays with Utf8JsonWriter, instead of boxing through an intermediate object graph. Same AOT/trim-safety, fewer allocations. MAP keys are written in Arrow source order. - Add ComplexTypeSerializingStream unit tests: STRUCT field order and null fields, nested array-in-struct, MAP ordering and non-string keys, null elements, the relaxed string encoder, multi-row buffer reuse, and non-complex column passthrough. The fractional-double assertion is framework-aware (.NET Core emits "1.1", .NET Framework "1.1000000000000001"). - Guard the telemetry serialization paths against drift: a round-trip test asserts the hand-written TelemetryPayloadWriter matches the reflection-based TelemetryJsonOptions, plus a reflection-based completeness check asserts the fixture populates every envelope property, so adding a property fails the build rather than silently dropping a field. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
I've rewritten the structured types JSON serializer to omit the intermediate .NET serialization and added new tests for it. I think the new version is more verifiably complete. |
Co-authored-by: eric-wang-1990 <115501094+eric-wang-1990@users.noreply.github.com>
…ream JSON serialization Add 14 new unit tests covering previously untested scalar type serialization paths in the complex-type JSON serializer: - Time32 (seconds, milliseconds) and Time64 (microseconds, nanoseconds): regression tests for the WriteRawValue -> WriteStringValue fix that produced invalid JSON for time values nested in complex types - Float, Int8, Int16, UInt8, UInt16, UInt32, UInt64: bare JSON numbers - Date64: quoted yyyy-MM-dd format - Binary: base64-encoded quoted string - DayTime and MonthDayNanosecond intervals: quoted interval strings Time tests include conditional expectations for both .NET 6+ (TimeOnly) and .NET Framework (TimeSpan) code paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
What's Changed
Adds net10.0 to the driver's target frameworks and enables the trim, single-file, and NativeAOT Roslyn analyzers for it (IsAotCompatible).
A normal
dotnet buildnow surfaces IL2xxx/IL3xxx warnings — which, with TreatWarningsAsErrors, fail the build — so the driver is verified AOT-ready without yet producing a native build.Enabling the analyzers flagged one category of problem: reflection-based System.Text.Json. Fixes:
Typeenum name collisions), even with a property-level [JsonConverter]. Serialize its small, stable envelope by hand (TelemetryPayloadWriter), reusing the proto converter for the leaf. The proto-free TelemetryRequest stays on source-gen; the reflection-based TelemetryJsonOptions is kept for the test suite.All four TFMs (netstandard2.0;net472;net8.0;net10.0) build clean with no IL/SYSLIB warnings; net10.0 goes from 28 IL warnings to 0. The 825 unit tests pass, including the telemetry tests that exercise the production serialization path.