Skip to content

Commit 4e55a24

Browse files
SteNicholasclaude
andcommitted
fix(types): keep child field metadata when serializing ARRAY/MAP types
ArrayType::ToJson and MapType::ToJson passed a null metadata when creating the DataType of their children, so DataType::Create could not recognise an extension type there. An ARRAY<VARIANT> therefore serialized as its physical ARRAY<ROW<value, metadata>>, and creating such a table failed when the schema was read back because that struct carries the fixed child field ids 0/1. Carry the child field metadata over instead. The metadata only ever selects VARIANT or BLOB in DataTypeToString, so ordinary element types serialize unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 727b1d3 commit 4e55a24

3 files changed

Lines changed: 43 additions & 3 deletions

File tree

src/paimon/common/types/array_type.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,10 @@ class ArrayType : public DataType {
4343
auto type = arrow::internal::checked_cast<arrow::ListType*>(type_.get());
4444
auto value_field = type->value_field();
4545

46+
// The element metadata is load-bearing: it is what marks an extension type such as
47+
// VARIANT.
4648
std::shared_ptr<DataType> data_type =
47-
DataType::Create(value_field->type(), value_field->nullable(), /*metadata=*/nullptr);
49+
DataType::Create(value_field->type(), value_field->nullable(), value_field->metadata());
4850
obj.AddMember(rapidjson::StringRef("element"),
4951
RapidJsonUtil::SerializeValue(*data_type, allocator).Move(), *allocator);
5052
return obj;

src/paimon/common/types/data_type_test.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
#include "gtest/gtest.h"
2323
#include "paimon/common/data/blob_utils.h"
2424
#include "paimon/common/data/variant/variant_type_utils.h"
25+
#include "paimon/common/types/data_field.h"
2526
#include "paimon/common/utils/date_time_utils.h"
27+
#include "paimon/common/utils/rapidjson_util.h"
2628

2729
namespace paimon::test {
2830

@@ -109,4 +111,38 @@ TEST(DataTypeTest, DataTypeToString) {
109111
ASSERT_THROW(dummy_data_type.DataTypeToString(unknown_type), std::invalid_argument);
110112
}
111113

114+
TEST(DataTypeTest, NestedTypeSerializationUsesChildMetadata) {
115+
// ARRAY and MAP must carry their children's metadata into the serialized type, because that
116+
// is what marks an extension type. Dropping it serialized a VARIANT as its physical
117+
// `struct<value, metadata>` ROW, whose fixed child ids 0/1 then broke reading the schema back.
118+
auto to_json = [](const std::shared_ptr<arrow::Field>& field) {
119+
auto data_type = DataType::Create(field->type(), field->nullable(), field->metadata());
120+
rapidjson::Document doc;
121+
auto value = data_type->ToJson(&doc.GetAllocator());
122+
rapidjson::StringBuffer buffer;
123+
rapidjson::Writer<rapidjson::StringBuffer> writer(buffer);
124+
value.Accept(writer);
125+
return std::string(buffer.GetString());
126+
};
127+
128+
auto variant_field = VariantTypeUtils::ToArrowField("element");
129+
auto array_field = arrow::field("arr", arrow::list(variant_field));
130+
ASSERT_EQ(to_json(array_field), R"({"type":"ARRAY","element":"VARIANT"})");
131+
132+
auto map_field =
133+
arrow::field("m", arrow::map(arrow::utf8(), VariantTypeUtils::ToArrowField("value")));
134+
ASSERT_EQ(to_json(map_field), R"({"type":"MAP","key":"STRING NOT NULL","value":"VARIANT"})");
135+
136+
// BLOB is marked the same way and was degraded to its physical BYTES the same way.
137+
auto blob_array = arrow::field("b", arrow::list(BlobUtils::ToArrowField("element", true)));
138+
ASSERT_EQ(to_json(blob_array), R"({"type":"ARRAY","element":"BLOB"})");
139+
140+
// Child metadata only ever selects an extension type, so ordinary elements are unaffected by
141+
// it being carried over.
142+
auto plain_child = arrow::field("element", arrow::int32(), /*nullable=*/true,
143+
arrow::KeyValueMetadata::Make({DataField::FIELD_ID}, {"7"}));
144+
ASSERT_EQ(to_json(arrow::field("a", arrow::list(plain_child))),
145+
R"({"type":"ARRAY","element":"INT"})");
146+
}
147+
112148
} // namespace paimon::test

src/paimon/common/types/map_type.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,15 @@ class MapType : public DataType {
4949
*allocator);
5050
auto type = arrow::internal::checked_cast<arrow::MapType*>(type_.get());
5151
auto key_field = type->key_field();
52+
// The key and value metadata is load-bearing: it is what marks an extension type such as
53+
// VARIANT.
5254
std::shared_ptr<DataType> key_data_type =
53-
DataType::Create(key_field->type(), key_field->nullable(), /*metadata=*/nullptr);
55+
DataType::Create(key_field->type(), key_field->nullable(), key_field->metadata());
5456
obj.AddMember(rapidjson::StringRef("key"),
5557
RapidJsonUtil::SerializeValue(*key_data_type, allocator).Move(), *allocator);
5658
auto value_field = type->item_field();
5759
std::shared_ptr<DataType> value_data_type =
58-
DataType::Create(value_field->type(), value_field->nullable(), /*metadata=*/nullptr);
60+
DataType::Create(value_field->type(), value_field->nullable(), value_field->metadata());
5961
obj.AddMember(rapidjson::StringRef("value"),
6062
RapidJsonUtil::SerializeValue(*value_data_type, allocator).Move(),
6163
*allocator);

0 commit comments

Comments
 (0)