GH-50993: [CI][Integration] Add extension-wrapped union to integration data by Alb3e3 · Pull Request #51027 · apache/arrow

Alb3e3 · 2026-08-28T19:22:47Z

Rationale for this change

Just as the C++ test suite gained coverage for extension-wrapped unions in #50927 (GH-50623), the cross-implementation integration tests should exercise this case so that every tested implementation handles an extension type whose storage is a union.

What changes are included in this PR?

A new archery integration datagen case, extension_union, with two columns:

sparse_union_ext: a sparse union (int32/utf8) wrapped in an extension type
dense_union_ext: a dense union (int16/binary) wrapped in an extension type

ExtensionField already delegates its storage type/children/generation to the wrapped field, so no new datagen machinery is needed.

Are these changes tested?

The generator produces valid integration JSON locally (schema carries the ARROW:extension:* metadata on the union-typed fields, children preserved, batch sizes [0, 7]). The cross-implementation integration matrix in CI is the real test here — I've intentionally added the case with no per-implementation skips so the matrix can show which implementations still need a temporary .skip_tester(...); I'll add those (with tracking links) based on the results.

Are there any user-facing changes?

No. This only adds integration-test data.

Closes#50993.

GitHub Issue: [CI][Integration] Add extension-wrapped union to integration data #50993

…gration data Add a new integration datagen case (extension_union) with sparse- and dense-union columns wrapped in an extension type, so the cross-language integration tests exercise the extension/union interaction. This mirrors the C++ IPC coverage added in apacheGH-50623 (apache#50927).

github-actions · 2026-08-28T19:23:14Z

⚠️ GitHub issue #50993has been automatically assigned in GitHub to PR creator.

The extension names in this case are not registered in the per-language integration binaries, so the C++ JSON reader (and others) rejected them with "Extension type not found". Flag them with ARROW:integration:allow_unregistered_extension so the extension metadata is preserved and the union storage is round-tripped by every implementation.

uros-b · 2026-08-29T21:13:40Z

Nice addition @Alb3e3!

alamb

Thanks @Alb3e3

alamb · 2026-08-30T10:26:34Z

 .skip_format(SKIP_FLIGHT, '.NET')
 .skip_tester('Ruby'),
+
+ generate_extension_wrapped_union_case(),


so does this mean that all the other implementations correctly handle this extension wrapped union? If so, maybe that implies that whatever the problem was in #50927 is C++ specific and maybe not worth additional cross implementation coverage

@alamb It depends how other implementations handle extension types. If they need to be registered as in Arrow C++ for wrapping to occur, then a bug could be hidden by lack of registration.

Confirmed. After removing the bypass, there were still two C++ integration consumers missing extension-type registration: flight-test-integration-client and c_data_integration_internal.
With just those registrations reverted locally, archery integration --run-flight --run-c-data -x -k extension_union fails in Flight with Extension type 'sparse-union-extension' not found while opening generated_extension_union.json.
I pushed f215787 to register the dense/sparse union extension types in both places, and rerunning the same focused extension_union Flight + C Data command passes locally.

pitrou · 2026-08-31T13:31:22Z

I've tested this without the fix from 3e6f730, and it still succeed.

I think this is due to the fact that the extension type is not registered, therefore there is no wrapping within ExtensionType that would exhibit the issue.

@Alb3e3 Can we remove ARROW:integration:allow_unregistered_extension?

Alb3e3 · 2026-08-31T14:43:05Z

Fixed in de746b3.

I removed ARROW:integration:allow_unregistered_extension and registered the existing dense_union_extension_type() / sparse_union_extension_type() test types in the C++ integration executable. The generated storage schemas and serialized metadata now match those registered types.

Removing the bypass also exposed a related integration JSON reader issue: validity parsing was selected from the logical EXTENSION type, so it incorrectly required VALIDITY for union storage. It now selects validity layout from the physical storage type, matching the IPC writer's behavior.

I verified the coverage is non-vacuous:

The C++ JSON reader reports both fields as extension<sparse-union-extension> / extension<dense-union-extension>.
JSON -> IPC -> validate -> JSON -> validate passes for the zero-length and populated batches.
Temporarily reverting only the writer fix from 3e6f730 makes validation fail on the populated sparse union: buffer ARROW-5: Update drill-fmpp-maven-plugin to 1.5.0 #1 is 0 bytes, expected at least 7 bytes.
arrow-json-integration-test passes and both C++ files are clang-format 18.1.8 clean.

pitrou · 2026-08-31T16:14:33Z

@Alb3e3 I think you also need to update the extension type registrations in the Flight integration test harness.

…sumers

Alb3e3 · 2026-08-31T18:44:09Z

The two failures on the previous head split cleanly:

AMD64 Conda C++ AVX2 was the same unrelated filesystem lane (arrow-filesystem-test / arrow-s3fs-module-test)
AMD64 Conda Integration Test exposed a real follow-up in the C Data export path for union-backed extensions

Root cause was cpp/src/arrow/c/bridge.cc deciding whether to drop the leading validity slot from the logical EXTENSION type instead of the physical storage type. That exported legacy-style extra buffers for union-backed extensions (3 instead of 2 for dense, 2 instead of 1 for sparse), which C++ import tolerated but Java/.NET/nanoarrow/Rust rejected.

I pushed b2b42d4 to switch that export decision to the storage layout and added a focused regression in arrow-c-bridge-test covering dense + sparse union extension exports. Local red/green on that test is now:

red on old code with the buffer-count mismatch above
green after the fix with arrow-c-bridge-test --gtest_filter='TestArrayExport.Extension'

CI has restarted from the new head.

pitrou · 2026-09-01T09:39:14Z

+ const DataType* physical_type = data->type.get();
+ if (physical_type->id() == Type::EXTENSION) {
+ physical_type =
+ checked_cast<const ExtensionType&>(*physical_type).storage_type().get();
+ }
+


I'm just noticing that we can simply call DataType::storage_id here:
constauto physical_type_id = data->type->storage_id();
and then:
if (n_buffers > 0 && !internal::may_have_validity_bitmap(physical_type_id)) {

pitrou · 2026-09-01T09:43:15Z

 TestPrimitive(ExampleSmallint);
 TestPrimitive(ExampleComplex128);
+ TestPrimitive([]() {
+ auto type = dense_union_extension_type();


Perhaps add helper functions ExampleDenseUnionExtension and ExampleSparseUnionExtension along with ExampleUuid etc.?

pitrou · 2026-09-01T09:44:35Z

+ ExtensionTypeGuard ext_guard({uuid(), dict_extension_type(),
+ dense_union_extension_type(),
+ sparse_union_extension_type()});


@paleolimbot @zeroshade I think similar guards will have to be added to arrow-go and nanoarrow so that the extension type wrapping happens in integration testing.

Thank you for the heads up!
nanoarrow doesn't have a registration mechanism (more like arrow-rs...it's all just metadata). Do I still need to do anything for this?

@paleolimbot Sorry for the misunderstanding. Then no, you don't have to do anything, I think :)

…ion example helpers - bridge.cc: replace the manual extension-storage unwrap with DataType::storage_id(), per review. - Add ExampleDenseUnionExtension/ExampleSparseUnionExtension to arrow/testing (extension_type.h + gtest_util.cc), alongside ExampleUuid et al., and use them in the bridge Extension export test instead of inline lambdas.

Alb3e3 · 2026-09-01T16:09:15Z

Thanks, both addressed in 3abb59a.

bridge.cc: replaced the manual extension-storage unwrap with data->type->storage_id(), and switched the two physical_type->id() checks to the resulting physical_type_id.
Added ExampleDenseUnionExtension / ExampleSparseUnionExtension to arrow/testing (declared in extension_type.h, defined in gtest_util.cc beside ExampleUuid et al.), and the export test now calls those instead of the inline lambdas.

On the two cross-implementation notes: agreed that arrow-go and nanoarrow will need the same wrapping guard in their integration consumers for the extension-wrapped union to be exercised there, and that a registration gap could otherwise hide a bug. That is outside this C++/Python change; I have left it for @paleolimbot / @zeroshade as you flagged rather than pull it into this PR.

Alb3e3 · 2026-09-01T16:13:53Z

Built and tested locally on macOS (Apple clang, bundled deps): the full arrow-c-bridge-test suite passes, 165/165. That covers TestArrayExport.Extension (which now drives the new ExampleDenseUnionExtension / ExampleSparseUnionExtension helpers) and all of TestArrayExport.* / TestArrayImport.*, so the storage_id() change on the export path is exercised across every array type, not just extensions.

pitrou

+1, thanks a lot for this @Alb3e3

Alb3e3 requested a review from pitrou as a code owner August 28, 2026 19:22

github-actionsBot added the awaiting review Awaiting review label Aug 28, 2026

uros-b approved these changes Aug 29, 2026
View reviewed changes

github-actionsBot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Aug 29, 2026

alamb reviewed Aug 30, 2026
View reviewed changes

github-actionsBot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Aug 30, 2026

apacheGH-50993: exercise registered union extensions
de746b3

github-actionsBot added awaiting change review Awaiting change review Component: C++ and removed awaiting changes Awaiting changes labels Aug 31, 2026

apacheGH-50993: Register union extension types in C++ integration con…
f215787
…sumers

Alb3e3 requested a review from lidavidm as a code owner August 31, 2026 17:52

github-actionsBot added the Component: FlightRPC label Aug 31, 2026

apacheGH-50993: Export union extension arrays with storage layout
b2b42d4

apacheGH-50993: Format bridge.cc
b6634be

pitrou reviewed Sep 1, 2026
View reviewed changes

Nit and lint
3282c05

pitrou approved these changes Sep 2, 2026
View reviewed changes

pitrou merged commit f69ec05 into apache:mainSep 2, 2026
60 of 61 checks passed

pitrou removed the awaiting change review Awaiting change review label Sep 2, 2026

pitrou mentioned this pull request Sep 2, 2026
[CI][Integration] Add extension-wrapped union to integration data #50993
Closed

github-actionsBot added the awaiting changes Awaiting changes label Sep 2, 2026

Uh oh!

Conversation

Alb3e3 commented Aug 28, 2026• edited by github-actions BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actionsBot commented Aug 28, 2026

Uh oh!

uros-b commented Aug 29, 2026

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pitrou commented Aug 31, 2026

Uh oh!

Alb3e3 commented Aug 31, 2026

Uh oh!

pitrou commented Aug 31, 2026

Uh oh!

Alb3e3 commented Aug 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Alb3e3 commented Sep 1, 2026

Uh oh!

Alb3e3 commented Sep 1, 2026

Uh oh!

pitrou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Alb3e3 commented Aug 28, 2026•
edited by github-actions Bot
Loading