feat(csharp): implement EnableComplexDatatypeSupport for consistent complex type behavior (PECO-2938)

[eric-wang-1990]

Summary

Implements JDBC-equivalent EnableComplexDatatypeSupport parameter that unifies complex type (ARRAY, MAP, STRUCT) behavior across Thrift and SEA protocols.

Problem

Previously, Thrift and SEA returned complex types differently:

• Thrift: JSON strings (e.g. [1,2,3], {"a":1}) via ComplexTypesAsArrow=false
• SEA: Native Arrow types (ListType, MapType, StructType)

This caused test divergence and required IsSeaMode branching.

Solution

Adds adbc.databricks.enable_complex_datatype_support parameter (default false) that makes both protocols behave consistently — matching JDBC's EnableComplexDatatypeSupport flag.

Default (false) — JSON strings from both protocols:

• Thrift: ComplexTypesAsArrow=false (server returns JSON strings natively)
• SEA: new ComplexTypeSerializingStream wraps result and serializes native Arrow types to JSON strings via ArrowComplexTypeJsonSerializer

When enabled (true) — native Arrow types from both protocols:

• Thrift: ComplexTypesAsArrow=true (server returns native Arrow types)
• SEA: native Arrow types returned as-is

New Files

• ArrowComplexTypeJsonSerializer.cs — serializes Arrow LIST/MAP/STRUCT values to JSON using Utf8JsonWriter, handles nested complex types
• ComplexTypeSerializingStream.cs — IArrowArrayStream wrapper that converts complex columns to StringArray when EnableComplexDatatypeSupport=false

Changes

• DatabricksParameters.cs — adds EnableComplexDatatypeSupport constant
• DatabricksConnection.cs — reads and exposes the parameter
• DatabricksStatement.cs — drives ComplexTypesAsArrow from the parameter
• StatementExecutionStatement.cs — wraps result stream with ComplexTypeSerializingStream when disabled
• ComplexTypesValueTests.cs — simplified to assert unified JSON string output without IsSeaMode branching

Test Plan

• All 466 unit tests pass
• E2E COMPLEX-001..014 tests pass against live Databricks (Thrift and SEA)
• Verify [1,2,3] JSON string output for ARRAY in both protocols
• Verify {"a":1,"b":2} JSON string output for MAP in both protocols
• Verify {"id":1,"name":"Alice"} JSON string output for STRUCT in both protocols

🤖 Generated with Claude Code

[msrathore-db]

Bug confirmed: non-string MAP keys silently coerced to "null" — data loss

I've confirmed the issue in ToMapDict with a unit test run against this branch (after merging main).

Root cause

In ComplexTypeSerializingStream.cs:

private static Dictionary<string, object?> ToMapDict(StructArray entries, int start, int end)
{
    var keyArray = entries.Fields[0];
    ...
    for (int i = start; i < end; i++)
    {
        string key = keyArray is StringArray sa ? sa.GetString(i) ?? "null" : "null";
        //                                                                    ^^^^^^
        //                  Any non-string key (INT, BIGINT, DATE...) falls here
    }
}

For any non-StringArray key type, every key becomes the literal string "null". When the map has more than one entry, subsequent entries silently overwrite earlier ones because they all share the same key — complete data loss.

Test proving it

[Fact]
public async Task MapWithInt32Keys_SerializesKeyAsString_NotNull()
{
    // MAP<INT, STRING> = {1: "one", 2: "two"}
    var mapType = new MapType(Int32Type.Default, StringType.Default, false);
    var builder = new MapArray.Builder(mapType);
    builder.Append();
    ((Int32Array.Builder)builder.KeyBuilder).Append(1).Append(2);
    ((StringArray.Builder)builder.ValueBuilder).Append("one").Append("two");
    var mapArray = builder.Build();

    string? json = await SerializeToStringAsync(mapArray);

    Assert.Equal("{\"1\":\"one\",\"2\":\"two\"}", json); // FAILS
}

Actual vs expected output

	Value
Actual (buggy)	{"null":"two"} — first entry erased
Expected	{"1":"one","2":"two"}

Both Int32 and Int64 key types are affected. The same issue applies to any future non-string key type (DATE, BOOLEAN, DECIMAL, etc.).

Fix

// Replace:
string key = keyArray is StringArray sa ? sa.GetString(i) ?? "null" : "null";

// With:
string key = ToObject(keyArray, i)?.ToString() ?? "null";

ToObject already handles all Arrow primitive types correctly (int, long, float, bool, date, timestamp, decimal) and is already defined in this class — no new dependencies needed.

The test file (csharp/test/Unit/ComplexTypeSerializingStreamTests.cs) is ready to be included in this PR to prevent regressions.

[msrathore-db]

Can you cover Interval datatype as a part of this PR?

[Copilot]

Pull request overview

Implements a new Databricks driver parameter (adbc.databricks.enable_complex_datatype_support, default false) to make complex type (ARRAY/MAP/STRUCT) results consistent between Thrift and Statement Execution (SEA) protocols, aligning with JDBC behavior.

Changes:

• Adds EnableComplexDatatypeSupport connection/statement parameter and wires it into Thrift (ComplexTypesAsArrow) and SEA (stream wrapper).
• Introduces ComplexTypeSerializingStream to serialize SEA complex Arrow types into JSON strings when the flag is disabled.
• Updates E2E complex-type tests to assert unified default JSON-string behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
csharp/src/DatabricksParameters.cs	Adds the new connection parameter constant and docs.
csharp/src/DatabricksConnection.cs	Parses/exposes EnableComplexDatatypeSupport from properties.
csharp/src/DatabricksStatement.cs	Drives Thrift ComplexTypesAsArrow from the new flag.
csharp/src/StatementExecution/StatementExecutionConnection.cs	Parses/exposes the flag for SEA connections.
csharp/src/StatementExecution/StatementExecutionStatement.cs	Wraps SEA result streams to serialize complex types when disabled.
csharp/src/ComplexTypeSerializingStream.cs	New stream wrapper converting complex columns to JSON strings.
csharp/test/E2E/ComplexTypesValueTests.cs	Reworks E2E assertions to expect JSON strings by default.
csharp/test/E2E/DatabricksTestEnvironment.cs	Adds SEA-protocol detection helper (IsSeaMode).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

When converting a complex field to StringType, the new field is always created with nullable: true, which can change schema nullability semantics compared to the original field. Consider preserving the original field’s nullability (e.g., use the original field’s nullable flag) while only changing the DataType.

[eric-wang-1990]

✅ Done — changed nullable: true to field.IsNullable to preserve the original field's nullability.

---

This comment was generated with GitHub MCP.

[Copilot]

The current JSON serialization builds Dictionary<string, object?> objects (for STRUCT/MAP) and then calls JsonSerializer.Serialize(...). Dictionary enumeration order is not guaranteed (notably on net472, which this project targets), so property ordering in the resulting JSON string can be nondeterministic and can break string-equality comparisons and clients that expect stable output. Prefer writing JSON with Utf8JsonWriter (or another ordered representation) to emit fields in schema/order deterministically.

Suggested change

	builder.AppendNull();
	else
	builder.Append(JsonSerializer.Serialize(ToObject(array, i)));
	{
	builder.AppendNull();
	}
	else
	{
	var value = ToObject(array, i);
	if (value is IDictionary<string, object?> dict)
	{
	// Wrap in a SortedDictionary to ensure deterministic key ordering
	var ordered = new SortedDictionary<string, object?>(dict);
	builder.Append(JsonSerializer.Serialize(ordered));
	}
	else
	{
	builder.Append(JsonSerializer.Serialize(value));
	}
	}

[eric-wang-1990]

✅ Done — changed ToMapDict to return SortedDictionary<string, object?> so MAP keys are always emitted in sorted order, giving deterministic JSON output across all target frameworks.

---

This comment was generated with GitHub MCP.

[Copilot]

This file uses several var locals (e.g., var batch, var fields, var indices) but the repo’s C# coding guidelines prefer explicit types over var (csharp/CODING_GUIDELINES.md). Consider switching these to explicit types for consistency across the codebase.

[eric-wang-1990]

✅ Done — replaced all var locals with explicit types throughout the file.

---

This comment was generated with GitHub MCP.

[Copilot]

The XML doc references docs/specs/complex-types.yaml, but that file/path doesn’t exist in this repo. Please update the reference to the correct spec/doc location (or add the missing doc if it’s supposed to be part of the repo).

Suggested change

	/// See docs/specs/complex-types.yaml for per-protocol type specifications.
	/// See the project documentation for per-protocol type specifications.

[eric-wang-1990]

✅ Done — replaced the broken docs/specs/complex-types.yaml reference with a note about EnableComplexDatatypeSupport=true behavior.

---

This comment was generated with GitHub MCP.

[Copilot]

Local variable arr is declared with var, but the repo’s C# coding guidelines prefer explicit types over var (see csharp/CODING_GUIDELINES.md). Consider changing this to an explicit StringArray declaration for consistency.

Suggested change

	var arr = (StringArray)batch.Column(0);
	StringArray arr = (StringArray)batch.Column(0);

[eric-wang-1990]

✅ Done — changed to StringArray arr.

---

This comment was generated with GitHub MCP.

[Copilot]

using Apache.Arrow.Ipc; appears unused in this file and will trigger CS8019; the repo treats warnings as errors, so this can break the build. Remove the unused using (or reference an Ipc type if it’s intended).

Suggested change

using Apache.Arrow.Ipc;

[eric-wang-1990]

✅ Done — removed using Apache.Arrow.Ipc; from both ComplexTypeSerializingStream.cs and ComplexTypesValueTests.cs.

---

This comment was generated with GitHub MCP.

[eric-wang-1990]

Reverting this — IArrowArrayStream is defined in Apache.Arrow.Ipc, so the using is required. Removing it caused a build failure (CS0246). Restored.

---

This comment was generated with GitHub MCP.

[Copilot]

MAP key handling currently falls back to the literal string "null" whenever the key column isn’t a StringArray (or contains null), which silently corrupts maps with non-string keys (e.g., MAP<INT, ...>) and can also collide with a real "null" key. Consider converting key values to a string representation (e.g., via ValueAt(...).ToString() with invariant culture) and decide explicitly how to handle null keys (reject/throw vs. serialize).

[eric-wang-1990]

✅ Done — fixed to ToObject(keyArray, i)?.ToString() ?? "null". This correctly handles all Arrow primitive key types (INT, BIGINT, DATE, BOOLEAN, DECIMAL, etc.) using the existing ToObject helper, eliminating the data loss bug with non-string keys.

---

This comment was generated with GitHub MCP.

[Copilot]

This XML doc for IsSeaMode describes SEA always returning complex types as native Arrow and Thrift returning JSON strings, but this PR introduces EnableComplexDatatypeSupport that changes the behavior (SEA can also return JSON strings when disabled). Update the doc to reflect the flag-dependent behavior to avoid misleading future test authors.

Suggested change

	/// In SEA mode, complex types (ARRAY, MAP, STRUCT) are returned as native Arrow types.
	/// In Thrift mode, they are returned as JSON-encoded strings.
	/// When complex datatype support is enabled (EnableComplexDatatypeSupport = true), SEA returns
	/// complex types (ARRAY, MAP, STRUCT) as native Arrow types. When it is disabled, SEA returns
	/// complex types as JSON-encoded strings, matching Thrift behavior. Thrift mode always returns
	/// complex types as JSON-encoded strings regardless of the flag.

[eric-wang-1990]

✅ Done — updated the doc to describe the flag-dependent behavior: SEA returns native Arrow types when EnableComplexDatatypeSupport=true, JSON strings when false (the default), and Thrift always returns JSON strings regardless.

---

This comment was generated with GitHub MCP.

[Copilot]

using Apache.Arrow.Ipc; appears unused in this test file and will trigger CS8019; the repo treats warnings as errors, so this can break the build. Remove the unused using (or reference an Ipc type if it’s intended).

Suggested change

using Apache.Arrow.Ipc;

[eric-wang-1990]

✅ Done — removed from ComplexTypesValueTests.cs.

---

This comment was generated with GitHub MCP.

[Copilot]

These tests only validate the default behavior (EnableComplexDatatypeSupport=false). Since this PR introduces a new flag with a non-default behavior (native Arrow complex types), consider adding coverage for EnableComplexDatatypeSupport=true as well (asserting ListType/MapType/StructType) to prevent regressions in the enabled code path.

[eric-wang-1990]

Good point. Adding EnableComplexDatatypeSupport=true coverage (asserting native ListType/MapType/StructType) would only be meaningful for SEA mode — Thrift always returns JSON strings regardless of the flag. We could add a SEA-only test that re-opens the connection with the flag enabled and asserts the native Arrow types. Deferring to a follow-up PR to keep this one focused, but open to adding it here if preferred.

---

This comment was generated with GitHub MCP.

[eric-wang-1990]

Interval is a primitive scalar type, not a complex type, so ComplexTypeSerializingStream doesn't touch it:

• Top-level INTERVAL column: completely unaffected by this class — SEA returns it as a native Arrow IntervalType regardless of EnableComplexDatatypeSupport. Nothing to cover here.
• INTERVAL nested inside a complex type (e.g. STRUCT<id INT, duration INTERVAL>): ToObject would fall through to array.ValueAt(index, StructResultType.Object). This is an edge case not commonly seen in Databricks SQL and warrants a separate investigation.

Tracking the nested-interval edge case in PECO-2961 for a follow-up.

[eric-wang-1990]

Somehow keep this comment

[eric-wang-1990]

✅ Done — restored an expanded comment on ComplexTypesAsArrow explaining:

• When false (default): Thrift server returns complex types as JSON-encoded strings
• When true: server returns native Arrow types
• Notes the ARRAY_TYPE limitation: element type info isn't embedded in Thrift responses, so string serialization is the safe default

---

This comment was generated with GitHub MCP.

[eric-wang-1990]

Where is this used?

[eric-wang-1990]

✅ Removed — after searching the codebase, IsSeaMode was never called anywhere. Removed the property and the now-unused using AdbcDrivers.Databricks.StatementExecution directive.

---

This comment was generated with GitHub MCP.