You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "CurtHagenlocher (via GitHub)" <gi...@apache.org> on 2026/10/18 23:46:56 UTC

[PR] Put dictionary value extension metadata on the C Data dictionary child [arrow-dotnet]

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

   ## What's Changed
   
   For a dictionary-encoded extension type such as `dictionary<int32, arrow.uuid>`, Arrow C++ puts the value type's `ARROW:extension:name` on the `dictionary` child `ArrowSchema`. `CArrowSchemaExporter` put it on the field instead, which is where IPC keeps it. As a result, pyarrow rejected such fields (`Invalid storage type for UuidType: dictionary<...>`), and the importer didn't resolve them in either direction (#473).
   
   In memory, .NET follows IPC: a `DictionaryType` whose value type is the extension type, with the extension name in the field metadata. The C Data exporter and importer now translate between that and the C Data placement.
   
   ### Export
   
   - For a dictionary field, the extension keys are written on the `dictionary` child, and any other field metadata stays on the field. This also applies when the keys come only from the field metadata, as with a field read from IPC without a registry.
   - `ExportType` now writes extension metadata when given an `ExtensionType`, as Arrow C++ does. The dictionary child is exported through it. **Behavior change:** `ExportType(GuidType.Default, ptr)` previously produced a bare `w:16` with no extension name.
   - An extension type whose storage is a dictionary keeps its metadata on the field, which is also how Arrow C++ encodes that case.
   
   ### Import
   
   - The `dictionary` child's extension metadata resolves the value type against the registry, and is copied into the field metadata so the result matches the IPC reader.
   - For schemas exported by earlier versions of this library, an extension name on a dictionary field is tried against the whole dictionary type first and then against the value type.
   
   ### Tests
   
   - .NET: where export puts the metadata (including moving field-level keys), schema round trips with and without a registry, the legacy field-level placement, and a `DictionaryArray` of `GuidArray` round trip.
   - pyarrow: .NET → pyarrow gives `dictionary<values=extension<arrow.uuid>, indices=int32>` with equal values, and pyarrow → .NET gives a `DictionaryArray` whose `Dictionary` is a `GuidArray`.
   
   Closes #473.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   
   Co-Authored-By: Claude Opus 5.5 <no...@anthropic.com>
   


-- 
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] Put dictionary value extension metadata on the C Data dictionary child [arrow-dotnet]

Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on code in PR #474:
URL: https://github.com/apache/arrow-dotnet/pull/474#discussion_r4309335632


##########
src/Apache.Arrow/C/CArrowSchemaExporter.cs:
##########
@@ -59,17 +67,25 @@ public static unsafe void ExportType(IArrowType datatype, CArrowSchema* schema)
                 throw new ArgumentNullException(nameof(schema));
             }
 
+            // A dictionary-encoded field's extension metadata describes its value type. IPC
+            // keeps it on the field, but the C Data Interface puts it on the dictionary child.
+            IReadOnlyDictionary<string, string> dictionaryMetadata = null;
+            if (datatype is DictionaryType)
+            {
+                metadata = SplitExtensionMetadata(metadata, out dictionaryMetadata);

Review Comment:
   Agreed, thanks. This was a real problem. On a dictionary field, C Data extension metadata means the extension's storage is the whole dictionary; that's how Arrow C++ exports an extension over `dictionary<int32, string>`. Moving unresolved keys to the dictionary child changed that meaning.
   
   Fixed in 4ad0d8b:
   - The exporter moves field-level extension keys to the dictionary child only when they name the resolved `ExtensionType` value type, which is how the IPC reader leaves them. Unresolved keys stay on the field.
   - The importer no longer copies dictionary child extension metadata to the field, since that would cause the same misplacement in reverse. Like Arrow C++, an unresolved value type's extension metadata is dropped.
   - Added `CDataDictionaryStorageExtensionPassThroughWithoutRegistry`, which imports an extension over `dictionary<int32, string>` without its definition, re-exports it, and checks that the metadata stays on the field. Also added `CDataExportKeepsUnresolvedDictionaryFieldExtensionMetadata`.
   
   IPC has the same ambiguity, and our IPC reader resolves it the opposite way from Arrow C++. That's filed as #475.



-- 
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] Put dictionary value extension metadata on the C Data dictionary child [arrow-dotnet]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #474:
URL: https://github.com/apache/arrow-dotnet/pull/474#discussion_r4309311180


##########
src/Apache.Arrow/C/CArrowSchemaExporter.cs:
##########
@@ -59,17 +67,25 @@ public static unsafe void ExportType(IArrowType datatype, CArrowSchema* schema)
                 throw new ArgumentNullException(nameof(schema));
             }
 
+            // A dictionary-encoded field's extension metadata describes its value type. IPC
+            // keeps it on the field, but the C Data Interface puts it on the dictionary child.
+            IReadOnlyDictionary<string, string> dictionaryMetadata = null;
+            if (datatype is DictionaryType)
+            {
+                metadata = SplitExtensionMetadata(metadata, out dictionaryMetadata);

Review Comment:
   This also moves metadata for an unresolved extension whose storage is a dictionary. For example, importing an extension over `dictionary<int32, string>` without its registry definition leaves a plain `DictionaryType` with the extension keys on the field. Re-exporting now puts those keys on the string-valued dictionary child, changing the extension's storage type and potentially causing the consumer to reject it. Preserve whether unresolved metadata describes the dictionary itself or its values, while retaining the IPC metadata relocation, and add a no-registry import/re-export regression 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] Put dictionary value extension metadata on the C Data dictionary child [arrow-dotnet]

Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher merged PR #474:
URL: https://github.com/apache/arrow-dotnet/pull/474


-- 
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