This is an automated email from the ASF dual-hosted git repository.
pitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new f69ec05524 GH-50993: [CI][Integration] Add extension-wrapped union to
integration data (#51027)
f69ec05524 is described below
commit f69ec05524b0d6ed44c3fa804377332dfc085fac
Author: alb3e3 <[email protected]>
AuthorDate: Wed Sep 2 16:37:38 2026 +0200
GH-50993: [CI][Integration] Add extension-wrapped union to integration data
(#51027)
### 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: #50993
Lead-authored-by: Alb3e3 <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/c/bridge.cc | 9 +++++---
cpp/src/arrow/c/bridge_test.cc | 10 ++++++++-
.../integration_tests/test_integration_client.cc | 5 +++--
.../integration/c_data_integration_internal.cc | 3 ++-
cpp/src/arrow/integration/json_integration_test.cc | 4 +++-
cpp/src/arrow/integration/json_internal.cc | 2 +-
cpp/src/arrow/testing/extension_type.h | 6 ++++++
cpp/src/arrow/testing/gtest_util.cc | 14 ++++++++++++
dev/archery/archery/integration/datagen.py | 25 ++++++++++++++++++++++
9 files changed, 69 insertions(+), 9 deletions(-)
diff --git a/cpp/src/arrow/c/bridge.cc b/cpp/src/arrow/c/bridge.cc
index 4391d4cbc2..184be3ab8e 100644
--- a/cpp/src/arrow/c/bridge.cc
+++ b/cpp/src/arrow/c/bridge.cc
@@ -579,16 +579,19 @@ struct ArrayExporter {
// This is because ARROW-9037 is in version 0.17 and 0.17.1, and they are
// not able to import arrays without a null bitmap and null_count == -1.
data->GetNullCount();
+
+ const auto physical_type_id = data->type->storage_id();
+
// Store buffer pointers
size_t n_buffers = data->buffers.size();
auto buffers_begin = data->buffers.begin();
- if (n_buffers > 0 &&
!internal::may_have_validity_bitmap(data->type->id())) {
+ if (n_buffers > 0 &&
!internal::may_have_validity_bitmap(physical_type_id)) {
--n_buffers;
++buffers_begin;
}
- bool need_variadic_buffer_sizes = data->type->storage_id() ==
Type::BINARY_VIEW ||
- data->type->storage_id() ==
Type::STRING_VIEW;
+ bool need_variadic_buffer_sizes =
+ physical_type_id == Type::BINARY_VIEW || physical_type_id ==
Type::STRING_VIEW;
if (need_variadic_buffer_sizes) {
++n_buffers;
}
diff --git a/cpp/src/arrow/c/bridge_test.cc b/cpp/src/arrow/c/bridge_test.cc
index 4372732675..9a4d104d00 100644
--- a/cpp/src/arrow/c/bridge_test.cc
+++ b/cpp/src/arrow/c/bridge_test.cc
@@ -575,9 +575,15 @@ struct ArrayExportChecker {
ASSERT_EQ(c_export->null_count, expected_data.null_count);
ASSERT_EQ(c_export->offset, expected_data.offset);
+ const DataType* physical_type = expected_data.type.get();
+ if (physical_type->id() == Type::EXTENSION) {
+ physical_type =
+ checked_cast<const
ExtensionType&>(*physical_type).storage_type().get();
+ }
+
auto expected_n_buffers =
static_cast<int64_t>(expected_data.buffers.size());
auto expected_buffers = expected_data.buffers.data();
- if (!internal::may_have_validity_bitmap(expected_data.type->id())) {
+ if (!internal::may_have_validity_bitmap(physical_type->id())) {
--expected_n_buffers;
++expected_buffers;
}
@@ -1173,6 +1179,8 @@ TEST_F(TestArrayExport, Extension) {
TestPrimitive(ExampleUuid);
TestPrimitive(ExampleSmallint);
TestPrimitive(ExampleComplex128);
+ TestPrimitive(ExampleDenseUnionExtension);
+ TestPrimitive(ExampleSparseUnionExtension);
}
TEST_F(TestArrayExport, MovePrimitive) {
diff --git a/cpp/src/arrow/flight/integration_tests/test_integration_client.cc
b/cpp/src/arrow/flight/integration_tests/test_integration_client.cc
index c5c3f10576..f39a4f04eb 100644
--- a/cpp/src/arrow/flight/integration_tests/test_integration_client.cc
+++ b/cpp/src/arrow/flight/integration_tests/test_integration_client.cc
@@ -145,8 +145,9 @@ class IntegrationTestScenario : public Scenario {
Status RunClient(std::unique_ptr<FlightClient> client) override {
// Make sure the required extension types are registered.
- ExtensionTypeGuard uuid_ext_guard(uuid());
- ExtensionTypeGuard dict_ext_guard(dict_extension_type());
+ ExtensionTypeGuard ext_guard({uuid(), dict_extension_type(),
+ dense_union_extension_type(),
+ sparse_union_extension_type()});
FlightDescriptor descr{FlightDescriptor::PATH, "", {FLAGS_path}};
diff --git a/cpp/src/arrow/integration/c_data_integration_internal.cc
b/cpp/src/arrow/integration/c_data_integration_internal.cc
index b21a0cc13b..80b1758086 100644
--- a/cpp/src/arrow/integration/c_data_integration_internal.cc
+++ b/cpp/src/arrow/integration/c_data_integration_internal.cc
@@ -38,7 +38,8 @@ namespace {
// Make sure the extension types referenced in test data are registered.
[[nodiscard]] auto RequireExtensionTypes() {
- return ExtensionTypeGuard({uuid(), dict_extension_type()});
+ return ExtensionTypeGuard({uuid(), dict_extension_type(),
dense_union_extension_type(),
+ sparse_union_extension_type()});
}
template <typename Func>
diff --git a/cpp/src/arrow/integration/json_integration_test.cc
b/cpp/src/arrow/integration/json_integration_test.cc
index 98bbd3f56b..0f7e6c85dc 100644
--- a/cpp/src/arrow/integration/json_integration_test.cc
+++ b/cpp/src/arrow/integration/json_integration_test.cc
@@ -225,7 +225,9 @@ Status RunCommand(const std::string& json_path, const
std::string& arrow_path,
const std::string& command) {
// Make sure the required extension types are registered, as they will be
// referenced in test data.
- ExtensionTypeGuard ext_guard({uuid(), dict_extension_type()});
+ ExtensionTypeGuard ext_guard({uuid(), dict_extension_type(),
+ dense_union_extension_type(),
+ sparse_union_extension_type()});
if (json_path == "") {
return Status::Invalid("Must specify json file name");
diff --git a/cpp/src/arrow/integration/json_internal.cc
b/cpp/src/arrow/integration/json_internal.cc
index 0f72207bd4..9dd9963cd5 100644
--- a/cpp/src/arrow/integration/json_internal.cc
+++ b/cpp/src/arrow/integration/json_internal.cc
@@ -1952,7 +1952,7 @@ class ArrayReader {
Result<std::shared_ptr<ArrayData>> Parse() {
ARROW_ASSIGN_OR_RAISE(length_, GetMemberInt<int32_t>(obj_, "count"));
- if (::arrow::internal::may_have_validity_bitmap(type_->id())) {
+ if (::arrow::internal::may_have_validity_bitmap(type_->storage_id())) {
// Null and union types don't have a validity bitmap
RETURN_NOT_OK(ParseValidityBitmap());
}
diff --git a/cpp/src/arrow/testing/extension_type.h
b/cpp/src/arrow/testing/extension_type.h
index e5d6b597c5..f931206a0d 100644
--- a/cpp/src/arrow/testing/extension_type.h
+++ b/cpp/src/arrow/testing/extension_type.h
@@ -266,6 +266,12 @@ std::shared_ptr<Array> ExampleDictExtension();
ARROW_TESTING_EXPORT
std::shared_ptr<Array> ExampleComplex128();
+ARROW_TESTING_EXPORT
+std::shared_ptr<Array> ExampleDenseUnionExtension();
+
+ARROW_TESTING_EXPORT
+std::shared_ptr<Array> ExampleSparseUnionExtension();
+
ARROW_TESTING_EXPORT
std::shared_ptr<Array> MakeComplex128(const std::shared_ptr<Array>& real,
const std::shared_ptr<Array>& imag);
diff --git a/cpp/src/arrow/testing/gtest_util.cc
b/cpp/src/arrow/testing/gtest_util.cc
index daadfe9c2c..b7d2a963d0 100644
--- a/cpp/src/arrow/testing/gtest_util.cc
+++ b/cpp/src/arrow/testing/gtest_util.cc
@@ -1092,6 +1092,20 @@ std::shared_ptr<Array> ExampleComplex128() {
return ExtensionType::WrapArray(complex128(), arr);
}
+std::shared_ptr<Array> ExampleDenseUnionExtension() {
+ auto type = dense_union_extension_type();
+ auto storage_type = checked_cast<const ExtensionType&>(*type).storage_type();
+ return ExtensionType::WrapArray(
+ type, ArrayFromJSON(storage_type, R"([[0, 1.5], [1, "abc"]])"));
+}
+
+std::shared_ptr<Array> ExampleSparseUnionExtension() {
+ auto type = sparse_union_extension_type();
+ auto storage_type = checked_cast<const ExtensionType&>(*type).storage_type();
+ return ExtensionType::WrapArray(
+ type, ArrayFromJSON(storage_type, R"([[0, 1.5], [1, "abc"]])"));
+}
+
ExtensionTypeGuard::ExtensionTypeGuard(const std::shared_ptr<DataType>& type)
: ExtensionTypeGuard(DataTypeVector{type}) {}
diff --git a/dev/archery/archery/integration/datagen.py
b/dev/archery/archery/integration/datagen.py
index ed50919dfc..adffb2a1b2 100644
--- a/dev/archery/archery/integration/datagen.py
+++ b/dev/archery/archery/integration/datagen.py
@@ -1938,6 +1938,29 @@ def generate_extension_case():
dictionaries=[dict0])
+def generate_extension_wrapped_union_case():
+ # Unions wrapped in an extension type, exercising the extension/union
+ # interaction across implementations (see the C++ fix in GH-50623).
+ sparse_union_type = ExtensionType(
+ 'sparse-union-extension', 'sparse-union-extension',
+ SparseUnionField('', [get_field('floats', 'float64'),
+ get_field('strings', 'largeutf8')],
+ type_ids=[0, 1]))
+ dense_union_type = ExtensionType(
+ 'dense-union-extension', 'dense-union-extension',
+ DenseUnionField('', [get_field('floats', 'float64'),
+ get_field('strings', 'largeutf8')],
+ type_ids=[0, 1]))
+
+ fields = [
+ ExtensionField('sparse_union_ext', sparse_union_type),
+ ExtensionField('dense_union_ext', dense_union_type),
+ ]
+
+ batch_sizes = [0, 7]
+ return _generate_file("extension_union", fields, batch_sizes)
+
+
def get_generated_json_files(tempdir=None):
tempdir = tempdir or tempfile.mkdtemp(prefix='arrow-integration-')
@@ -2047,6 +2070,8 @@ def get_generated_json_files(tempdir=None):
# TODO(https://github.com/apache/arrow/issues/38045)
.skip_format(SKIP_FLIGHT, '.NET')
.skip_tester('Ruby'),
+
+ generate_extension_wrapped_union_case(),
]
generated_paths = []
