This is an automated email from the ASF dual-hosted git repository.
wgtmac pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new 4b9ff79f fix(arrow): release schema after conversion failure (#862)
4b9ff79f is described below
commit 4b9ff79fda7a66d4cf4c44d759334f5d04ed5eb5
Author: wzhuo <[email protected]>
AuthorDate: Mon Aug 3 11:55:00 2026 +0800
fix(arrow): release schema after conversion failure (#862)
Release partially initialized ArrowSchema output when internal
Iceberg-to-Arrow conversion fails. Add coverage using fixed(0), which
passes compatibility validation but is rejected by nanoarrow, and verify
the output release callback is cleared. Tests: arrow_test.
---
src/iceberg/arrow_c_data_guard_internal.h | 6 ++++++
src/iceberg/schema_internal.cc | 6 ++++--
src/iceberg/test/arrow_test.cc | 13 ++++++++++++-
3 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/src/iceberg/arrow_c_data_guard_internal.h
b/src/iceberg/arrow_c_data_guard_internal.h
index f624f74c..1b1417a9 100644
--- a/src/iceberg/arrow_c_data_guard_internal.h
+++ b/src/iceberg/arrow_c_data_guard_internal.h
@@ -46,6 +46,12 @@ class ICEBERG_EXPORT ArrowSchemaGuard {
explicit ArrowSchemaGuard(ArrowSchema* schema) : schema_(schema) {}
~ArrowSchemaGuard();
+ /// \brief Release the guard without calling ArrowSchemaRelease.
+ ///
+ /// Call this when ownership of the underlying ArrowSchema has been
+ /// transferred elsewhere and the guard should not release it.
+ void Release() { schema_ = nullptr; }
+
private:
ArrowSchema* schema_;
};
diff --git a/src/iceberg/schema_internal.cc b/src/iceberg/schema_internal.cc
index 5dac0d3b..810e5369 100644
--- a/src/iceberg/schema_internal.cc
+++ b/src/iceberg/schema_internal.cc
@@ -25,6 +25,7 @@
#include <optional>
#include <string>
+#include "iceberg/arrow_c_data_guard_internal.h"
#include "iceberg/constants.h"
#include "iceberg/schema.h"
#include "iceberg/type.h"
@@ -71,6 +72,7 @@ ArrowErrorCode ToArrowSchema(const Type& type, bool optional,
std::string_view n
std::optional<int32_t> field_id, ArrowSchema*
schema) {
ArrowBuffer metadata_buffer;
NANOARROW_RETURN_NOT_OK(ArrowMetadataBuilderInit(&metadata_buffer, nullptr));
+ internal::ArrowArrayBufferGuard metadata_buffer_guard(&metadata_buffer);
if (field_id.has_value()) {
NANOARROW_RETURN_NOT_OK(ArrowMetadataBuilderAppend(
&metadata_buffer,
ArrowCharView(std::string(kParquetFieldIdKey).c_str()),
@@ -183,7 +185,6 @@ ArrowErrorCode ToArrowSchema(const Type& type, bool
optional, std::string_view n
case TypeId::kVariant:
case TypeId::kGeometry:
case TypeId::kGeography:
- ArrowBufferReset(&metadata_buffer);
return EINVAL;
}
@@ -193,7 +194,6 @@ ArrowErrorCode ToArrowSchema(const Type& type, bool
optional, std::string_view n
NANOARROW_RETURN_NOT_OK(ArrowSchemaSetMetadata(
schema, reinterpret_cast<const char*>(metadata_buffer.data)));
- ArrowBufferReset(&metadata_buffer);
if (optional) {
schema->flags |= ARROW_FLAG_NULLABLE;
@@ -214,6 +214,7 @@ Status ToArrowSchema(const Schema& schema, ArrowSchema*
out) {
ICEBERG_RETURN_UNEXPECTED(CheckArrowCompatible(schema));
ArrowSchemaInit(out);
+ internal::ArrowSchemaGuard schema_guard(out);
if (ArrowErrorCode errorCode = ToArrowSchema(schema, /*optional=*/false,
/*name=*/"",
/*field_id=*/std::nullopt, out);
@@ -222,6 +223,7 @@ Status ToArrowSchema(const Schema& schema, ArrowSchema*
out) {
"Failed to convert Iceberg schema to Arrow schema, error code: {}",
errorCode);
}
+ schema_guard.Release();
return {};
}
diff --git a/src/iceberg/test/arrow_test.cc b/src/iceberg/test/arrow_test.cc
index d18a6eaf..1ba95cdd 100644
--- a/src/iceberg/test/arrow_test.cc
+++ b/src/iceberg/test/arrow_test.cc
@@ -130,12 +130,23 @@ TEST(ToArrowSchemaTest, UnsupportedV3Types) {
Schema schema(
{SchemaField::MakeOptional(/*field_id=*/1, "unsupported",
unsupported_type)},
/*schema_id=*/0);
- ArrowSchema arrow_schema;
+ ArrowSchema arrow_schema{};
ASSERT_THAT(ToArrowSchema(schema, &arrow_schema),
HasErrorMessage("is not supported by Arrow conversion"));
+ EXPECT_EQ(arrow_schema.release, nullptr);
}
}
+TEST(ToArrowSchemaTest, ReleasesSchemaOnConversionFailure) {
+ // fixed(0) passes CheckArrowCompatible but nanoarrow rejects its
non-positive width.
+ Schema schema({SchemaField::MakeOptional(/*field_id=*/1, "invalid_fixed",
fixed(0))},
+ /*schema_id=*/0);
+ ArrowSchema arrow_schema{};
+
+ EXPECT_THAT(ToArrowSchema(schema, &arrow_schema),
IsError(ErrorKind::kInvalidSchema));
+ EXPECT_EQ(arrow_schema.release, nullptr);
+}
+
namespace {
void CheckArrowField(const ::arrow::Field& field, ::arrow::Type::type type_id,
