diff --git a/docs/source/user_guide/data_types.rst b/docs/source/user_guide/data_types.rst index c1bdda4a0..17fdaca14 100644 --- a/docs/source/user_guide/data_types.rst +++ b/docs/source/user_guide/data_types.rst @@ -271,7 +271,10 @@ and `Arrow DataTypes `_ specification by setting ``variant.shreddingSchema`` to a ROW type JSON whose fields map top-level variant column names to their shredding - types. Alternatively, setting ``variant.inferShreddingSchema`` to + types. Explicit field IDs are preserved. If every ROW field, including + nested fields, omits ``id``, IDs are assigned in preorder starting at 0, + as in Java Paimon. Partially specified IDs are rejected. + Alternatively, setting ``variant.inferShreddingSchema`` to ``true`` infers a shredding schema per file from the first written rows (tuned by ``variant.shredding.maxSchemaWidth``, which bounds the total number of shredded fields across all variant columns of the schema, diff --git a/include/paimon/defs.h b/include/paimon/defs.h index f51eb4520..517ddb8b6 100644 --- a/include/paimon/defs.h +++ b/include/paimon/defs.h @@ -511,7 +511,9 @@ struct PAIMON_EXPORT Options { static const char MAP_SHARED_SHREDDING_COLUMN_PLACEMENT_POLICY[]; /// "variant.shreddingSchema" - The Variant shredding schema for writing: a ROW type JSON - /// whose fields map variant column names to their shredding types. No default value. + /// whose fields map variant column names to their shredding types. All ROW fields, including + /// nested fields, must all specify 'id' or all omit it. Omitted IDs are assigned in preorder + /// starting at 0. No default value. static const char VARIANT_SHREDDING_SCHEMA[]; /// "parquet.variant.shreddingSchema" - Fallback key of "variant.shreddingSchema". static const char PARQUET_VARIANT_SHREDDING_SCHEMA[]; diff --git a/src/paimon/common/data/variant/variant_shredding_write_plan.h b/src/paimon/common/data/variant/variant_shredding_write_plan.h index 2fa7a886e..c9903fe02 100644 --- a/src/paimon/common/data/variant/variant_shredding_write_plan.h +++ b/src/paimon/common/data/variant/variant_shredding_write_plan.h @@ -75,7 +75,8 @@ class VariantShreddingWritePlan { /// Creates a plan from the `variant.shreddingSchema` option value: a ROW type JSON whose /// fields map top-level variant column names to their shredding types (nested variant - /// columns cannot be configured, as in Java). + /// columns cannot be configured, as in Java). All ROW fields, including nested fields, must + /// all specify 'id' or all omit it. Omitted IDs are assigned in preorder starting at 0. static Result> FromConfiguredSchema( const std::shared_ptr& logical_schema, const std::string& configured_schema_json); diff --git a/src/paimon/common/data/variant/variant_shredding_write_plan_factory_test.cpp b/src/paimon/common/data/variant/variant_shredding_write_plan_factory_test.cpp index 1546acade..49b28134f 100644 --- a/src/paimon/common/data/variant/variant_shredding_write_plan_factory_test.cpp +++ b/src/paimon/common/data/variant/variant_shredding_write_plan_factory_test.cpp @@ -95,20 +95,63 @@ TEST_F(VariantShreddingWritePlanFactoryTest, ConfiguredSchema) { } } ] })"; - ASSERT_OK_AND_ASSIGN(CoreOptions options, - MakeOptions({{"variant.shreddingSchema", shredding_schema_json}})); - auto factory = VariantShreddingWritePlanFactory::Create(options, schema_, pool_); - ASSERT_TRUE(factory->ShouldCreateWritePlan()); - ASSERT_FALSE(factory->ShouldInferWritePlan()); - ASSERT_OK_AND_ASSIGN(std::shared_ptr converter, - factory->CreateConverter("parquet", {})); - ASSERT_NE(converter, nullptr); - auto variant_field = converter->GetPhysicalSchema()->GetFieldByName("v"); - ASSERT_NE(variant_field, nullptr); - const auto& physical_type = static_cast(*variant_field->type()); - ASSERT_NE(physical_type.GetFieldByName("typed_value"), nullptr); - // Variant shredding only supports the parquet format. - ASSERT_TRUE(factory->CreateConverter("orc", {}).status().IsNotImplemented()); + const char* shredding_schema_json_without_ids = R"({ + "type": "ROW", + "fields": [ { + "name": "v", + "type": { + "type": "ROW", + "fields": [ + {"name": "age", "type": "INT"}, + {"name": "city", "type": "STRING"} + ] + } + } ] + })"; + std::shared_ptr physical_schema; + for (const char* option_key : {"variant.shreddingSchema", "parquet.variant.shreddingSchema"}) { + SCOPED_TRACE(option_key); + for (const char* configured_schema : + {shredding_schema_json, shredding_schema_json_without_ids}) { + SCOPED_TRACE(configured_schema); + ASSERT_OK_AND_ASSIGN(CoreOptions options, + MakeOptions({{option_key, configured_schema}})); + auto factory = VariantShreddingWritePlanFactory::Create(options, schema_, pool_); + ASSERT_TRUE(factory->ShouldCreateWritePlan()); + ASSERT_FALSE(factory->ShouldInferWritePlan()); + ASSERT_OK_AND_ASSIGN(std::shared_ptr converter, + factory->CreateConverter("parquet", {})); + ASSERT_NE(converter, nullptr); + auto variant_field = converter->GetPhysicalSchema()->GetFieldByName("v"); + ASSERT_NE(variant_field, nullptr); + const auto& physical_type = + static_cast(*variant_field->type()); + ASSERT_NE(physical_type.GetFieldByName("typed_value"), nullptr); + if (physical_schema == nullptr) { + physical_schema = converter->GetPhysicalSchema(); + } else { + ASSERT_TRUE(converter->GetPhysicalSchema()->Equals(*physical_schema, + /*check_metadata=*/true)); + } + ASSERT_TRUE(factory->CreateConverter("orc", {}).status().IsNotImplemented()); + } + } +} + +TEST_F(VariantShreddingWritePlanFactoryTest, ConfiguredSchemaRejectsPartialFieldIds) { + for (const char* configured_schema : { + R"({"type":"ROW","fields":[{"id":0,"name":"v","type":{ + "type":"ROW","fields":[{"name":"age","type":"INT"}]}}]})", + R"({"type":"ROW","fields":[{"name":"v","type":{ + "type":"ROW","fields":[{"id":1,"name":"age","type":"INT"}]}}]})", + }) { + SCOPED_TRACE(configured_schema); + ASSERT_OK_AND_ASSIGN(CoreOptions options, + MakeOptions({{"variant.shreddingSchema", configured_schema}})); + auto factory = VariantShreddingWritePlanFactory::Create(options, schema_, pool_); + ASSERT_NOK_WITH_MSG(factory->CreateConverter("parquet", {}), + "Partial field id is not allowed."); + } } TEST_F(VariantShreddingWritePlanFactoryTest, InferredSchema) { diff --git a/src/paimon/common/types/data_field.cpp b/src/paimon/common/types/data_field.cpp index 97a167b0d..8323fcf0b 100644 --- a/src/paimon/common/types/data_field.cpp +++ b/src/paimon/common/types/data_field.cpp @@ -65,21 +65,13 @@ rapidjson::Value DataField::ToJson(rapidjson::Document::AllocatorType* allocator } void DataField::FromJson(const rapidjson::Value& obj) noexcept(false) { - id_ = RapidJsonUtil::DeserializeKeyValue(obj, "id"); - auto name = RapidJsonUtil::DeserializeKeyValue(obj, "name"); - assert(obj.IsObject()); - if (!obj.HasMember("type")) { - throw std::invalid_argument("key 'type' must exist"); - } - auto field_result = DataTypeJsonParser::ParseType(name, obj["type"]); + // Serialized table schemas require explicit IDs for projection and schema evolution. + Result field_result = DataTypeJsonParser::ParseDataField(obj); if (!field_result.ok()) { - throw std::invalid_argument( - fmt::format("parse data type failed, error msg: {}", field_result.status().ToString())); + throw std::invalid_argument(field_result.status().ToString()); } - field_ = field_result.value(); + *this = std::move(field_result).value(); assert(field_); - description_ = RapidJsonUtil::DeserializeKeyValue>( - obj, "description", description_); } std::shared_ptr DataField::ConvertDataFieldToArrowField(const DataField& field) { diff --git a/src/paimon/common/types/data_field_test.cpp b/src/paimon/common/types/data_field_test.cpp index 6ac52ff59..27841aaa9 100644 --- a/src/paimon/common/types/data_field_test.cpp +++ b/src/paimon/common/types/data_field_test.cpp @@ -20,6 +20,7 @@ #include "paimon/common/types/data_field.h" #include +#include #include "arrow/api.h" #include "gtest/gtest.h" @@ -137,7 +138,8 @@ TEST_F(DataFieldTest, ConvertArrowFieldToDataField) { TEST_F(DataFieldTest, FromJson) { const char* json = R"({ "id" : 0, - "name" : "f0", + "name" : "f0\u0000tail", + "description" : "d\u0000tail", "type" : { "type" : "ROW", "fields" : [ { @@ -161,7 +163,8 @@ TEST_F(DataFieldTest, FromJson) { DataField field; field.FromJson(doc); EXPECT_EQ(field.Id(), 0); - EXPECT_EQ(field.Name(), "f0"); + ASSERT_EQ(field.Name(), std::string("f0\0tail", 7)); + ASSERT_EQ(field.Description(), std::string("d\0tail", 6)); EXPECT_EQ(field.Type()->id(), arrow::Type::STRUCT); auto sub_fields = field.Type()->fields(); @@ -226,6 +229,30 @@ TEST_F(DataFieldTest, FromJsonFailed) { })"; check_result(json_str, "parse data type failed, error msg: "); } + const std::vector> test_cases = { + {R"([{"id":0,"name":"a","type":"INT"}])", "data field must be an object"}, + {R"({"id":"0","name":"a","type":"INT"})", "value of key 'id' must be int"}, + {R"({"name":"a","type":"INT"})", "key 'id' must exist"}, + {R"({"id":null,"name":"a","type":"INT"})", "key 'id' must exist"}, + {R"({"id":0,"type":"INT"})", "key 'name' must exist"}, + {R"({"id":0,"name":0,"type":"INT"})", "value of key 'name' must be string"}, + {R"({"id":0,"name":"a"})", "key 'type' must exist"}, + {R"({"id":0,"name":"a","type":"INT","description":0})", + "value of key 'description' must be string"}, + {R"({"id":0,"name":"a","type":{"type":"ROW", + "fields":[{"name":"b","type":"INT"}]}})", + "key 'id' must exist"}, + {R"({"id":0,"name":"a","type":{"type":"ARRAY","element":{"type":"ROW", + "fields":[{"name":"b","type":"INT"}]}}})", + "key 'id' must exist"}, + {R"({"id":0,"name":"a","type":{"type":"MAP","key":"STRING", + "value":{"type":"ROW","fields":[{"name":"b","type":"INT"}]}}})", + "key 'id' must exist"}, + }; + for (const auto& [json, error_msg] : test_cases) { + SCOPED_TRACE(json); + ASSERT_NO_FATAL_FAILURE(check_result(json, error_msg)); + } } TEST_F(DataFieldTest, ToJson) { diff --git a/src/paimon/common/types/data_type_json_parser.cpp b/src/paimon/common/types/data_type_json_parser.cpp index d04e5ade5..adcf5b73a 100644 --- a/src/paimon/common/types/data_type_json_parser.cpp +++ b/src/paimon/common/types/data_type_json_parser.cpp @@ -35,7 +35,6 @@ #include "paimon/common/types/data_field.h" #include "paimon/common/types/vector_type.h" #include "paimon/common/utils/date_time_utils.h" -#include "paimon/common/utils/rapidjson_util.h" #include "paimon/common/utils/string_utils.h" #include "paimon/data/decimal.h" #include "paimon/data/timestamp.h" @@ -647,17 +646,93 @@ Result TokenParser::ParseOptionalPrecision(int32_t default_precision) { } } // namespace +Result DataTypeJsonParser::FieldIdAssigner::Assign( + const std::optional& explicit_id) { + // Mixing both forms would let a generated id collide with an explicit one. + if (explicit_id.has_value()) { + if (next_generated_id_ != 0) { + return Status::Invalid("Partial field id is not allowed."); + } + has_explicit_id_ = true; + return explicit_id.value(); + } + if (has_explicit_id_) { + return Status::Invalid("Partial field id is not allowed."); + } + return next_generated_id_++; +} + Result> DataTypeJsonParser::ParseType( const std::string& name, const rapidjson::Value& type_json_value) { + FieldIdAssigner field_id_assigner; + return ParseType(name, type_json_value, &field_id_assigner); +} + +Result> DataTypeJsonParser::ParseType( + const std::string& name, const rapidjson::Value& type_json_value, + FieldIdAssigner* field_id_assigner) { if (type_json_value.IsString()) { return ParseAtomicTypeField(name, type_json_value); } else if (type_json_value.IsObject()) { - return ParseComplexTypeField(name, type_json_value); + return ParseComplexTypeField(name, type_json_value, field_id_assigner); } return Status::Invalid("cannot parse data type"); } +Result DataTypeJsonParser::ParseDataField(const rapidjson::Value& field_json_value) { + return ParseDataField(field_json_value, /*field_id_assigner=*/nullptr); +} + +Result DataTypeJsonParser::ParseDataField(const rapidjson::Value& field_json_value, + FieldIdAssigner* field_id_assigner) { + if (!field_json_value.IsObject()) { + return Status::Invalid("data field must be an object"); + } + std::optional explicit_id; + if (field_json_value.HasMember("id") && !field_json_value["id"].IsNull()) { + if (!field_json_value["id"].IsInt()) { + return Status::Invalid("value of key 'id' must be int"); + } + explicit_id = field_json_value["id"].GetInt(); + } + int32_t id = -1; + if (field_id_assigner != nullptr) { + PAIMON_ASSIGN_OR_RAISE(id, field_id_assigner->Assign(explicit_id)); + } else if (explicit_id.has_value()) { + id = explicit_id.value(); + } else { + return Status::Invalid("key 'id' must exist"); + } + if (!field_json_value.HasMember("name")) { + return Status::Invalid("key 'name' must exist"); + } + const auto& name_value = field_json_value["name"]; + if (!name_value.IsString()) { + return Status::Invalid("value of key 'name' must be string"); + } + std::string name(name_value.GetString(), name_value.GetStringLength()); + if (!field_json_value.HasMember("type")) { + return Status::Invalid("key 'type' must exist"); + } + Result> field_result = + ParseType(name, field_json_value["type"], field_id_assigner); + if (!field_result.ok()) { + return Status::Invalid( + fmt::format("parse data type failed, error msg: {}", field_result.status().ToString())); + } + std::optional description; + if (field_json_value.HasMember("description") && !field_json_value["description"].IsNull()) { + const auto& description_value = field_json_value["description"]; + if (!description_value.IsString()) { + return Status::Invalid("value of key 'description' must be string"); + } + description = + std::string(description_value.GetString(), description_value.GetStringLength()); + } + return DataField(id, field_result.value(), description); +} + Result> DataTypeJsonParser::ParseAtomicTypeField( const std::string& name, const rapidjson::Value& type_json_value) { bool nullable = true; @@ -674,7 +749,8 @@ Result> DataTypeJsonParser::ParseAtomicTypeField( } Result> DataTypeJsonParser::ParseComplexTypeField( - const std::string& name, const rapidjson::Value& type_json_value) { + const std::string& name, const rapidjson::Value& type_json_value, + FieldIdAssigner* field_id_assigner) { if (!type_json_value.HasMember("type")) { return Status::Invalid("complex data type must have type"); } @@ -686,13 +762,13 @@ Result> DataTypeJsonParser::ParseComplexTypeField( } if (StringUtils::StartsWith(type_str, "ARRAY")) { - return ParseArrayType(name, type_json_value, nullable); + return ParseArrayType(name, type_json_value, nullable, field_id_assigner); } else if (StringUtils::StartsWith(type_str, "VECTOR")) { - return ParseVectorType(name, type_json_value, nullable); + return ParseVectorType(name, type_json_value, nullable, field_id_assigner); } else if (StringUtils::StartsWith(type_str, "MAP")) { - return ParseMapType(name, type_json_value, nullable); + return ParseMapType(name, type_json_value, nullable, field_id_assigner); } else if (StringUtils::StartsWith(type_str, "ROW")) { - return ParseRowType(name, type_json_value, nullable); + return ParseRowType(name, type_json_value, nullable, field_id_assigner); } else if (StringUtils::StartsWith(type_str, "MULTISET")) { return Status::NotImplemented("MULTISET is not supported"); } @@ -701,18 +777,20 @@ Result> DataTypeJsonParser::ParseComplexTypeField( } Result> DataTypeJsonParser::ParseArrayType( - const std::string& name, const rapidjson::Value& type_json_value, bool nullable) { + const std::string& name, const rapidjson::Value& type_json_value, bool nullable, + FieldIdAssigner* field_id_assigner) { if (!type_json_value.HasMember("element")) { return Status::Invalid("array data type must have element"); } PAIMON_ASSIGN_OR_RAISE(std::shared_ptr element_field, - ParseType("item", type_json_value["element"])); + ParseType("item", type_json_value["element"], field_id_assigner)); return arrow::field(name, arrow::list(element_field), nullable); } Result> DataTypeJsonParser::ParseVectorType( - const std::string& name, const rapidjson::Value& type_json_value, bool nullable) { + const std::string& name, const rapidjson::Value& type_json_value, bool nullable, + FieldIdAssigner* field_id_assigner) { if (!type_json_value.HasMember("element") || !type_json_value.HasMember("length")) { return Status::Invalid("vector data type must have element and length"); } @@ -724,7 +802,7 @@ Result> DataTypeJsonParser::ParseVectorType( return Status::Invalid("Vector length must be between 1 and 2147483647 (both inclusive)"); } PAIMON_ASSIGN_OR_RAISE(std::shared_ptr element_field, - ParseType("item", type_json_value["element"])); + ParseType("item", type_json_value["element"], field_id_assigner)); if (!VectorType::IsValidElementType(element_field->type())) { return Status::Invalid( fmt::format("Invalid element type for vector: {}", element_field->type()->ToString())); @@ -733,13 +811,14 @@ Result> DataTypeJsonParser::ParseVectorType( } Result> DataTypeJsonParser::ParseMapType( - const std::string& name, const rapidjson::Value& type_json_value, bool nullable) { + const std::string& name, const rapidjson::Value& type_json_value, bool nullable, + FieldIdAssigner* field_id_assigner) { if (!type_json_value.HasMember("key") || !type_json_value.HasMember("value")) { return Status::Invalid("map data type must have key and value"); } PAIMON_ASSIGN_OR_RAISE(std::shared_ptr key, - ParseType("key", type_json_value["key"])); + ParseType("key", type_json_value["key"], field_id_assigner)); // NOTE: Unlike Java Paimon, this C++ implementation does not support nullable keys in // MapType. This is a limitation of Apache Arrow, which does not allow null keys in its @@ -748,14 +827,24 @@ Result> DataTypeJsonParser::ParseMapType( // interoperating with Java Paimon. key = key->WithNullable(false); PAIMON_ASSIGN_OR_RAISE(std::shared_ptr value, - ParseType("value", type_json_value["value"])); + ParseType("value", type_json_value["value"], field_id_assigner)); return arrow::field(name, std::make_shared(key, value), nullable); } Result> DataTypeJsonParser::ParseRowType( - const std::string& name, const rapidjson::Value& type_json_value, bool nullable) { - auto data_fields = - RapidJsonUtil::DeserializeKeyValue>(type_json_value, "fields"); + const std::string& name, const rapidjson::Value& type_json_value, bool nullable, + FieldIdAssigner* field_id_assigner) { + if (!type_json_value.HasMember("fields") || !type_json_value["fields"].IsArray()) { + return Status::Invalid("row data type must have fields"); + } + const auto& field_json_array = type_json_value["fields"].GetArray(); + std::vector data_fields; + data_fields.reserve(field_json_array.Size()); + for (const auto& field_json_value : field_json_array) { + PAIMON_ASSIGN_OR_RAISE(DataField data_field, + ParseDataField(field_json_value, field_id_assigner)); + data_fields.push_back(std::move(data_field)); + } auto struct_type = DataField::ConvertDataFieldsToArrowStructType(data_fields); return arrow::field(name, struct_type, nullable); diff --git a/src/paimon/common/types/data_type_json_parser.h b/src/paimon/common/types/data_type_json_parser.h index 2134236a7..ae302454e 100644 --- a/src/paimon/common/types/data_type_json_parser.h +++ b/src/paimon/common/types/data_type_json_parser.h @@ -19,12 +19,13 @@ #pragma once +#include #include +#include #include #include "arrow/api.h" #include "paimon/common/types/data_field.h" -#include "paimon/common/utils/rapidjson_util.h" #include "paimon/result.h" #include "rapidjson/document.h" @@ -34,7 +35,8 @@ class DataTypeJsonParser { DataTypeJsonParser() = delete; ~DataTypeJsonParser() = delete; - /// Parses a data type from a JSON value and returns an Arrow field representation. + /// Parses JSON into an Arrow field. If all ROW fields omit 'id', assigns IDs in preorder + /// starting at 0. Preserves explicit IDs and rejects partially specified IDs. /// /// @param name The name of the field. /// @param type_json_value The JSON value representing the type. @@ -42,20 +44,49 @@ class DataTypeJsonParser { static Result> ParseType(const std::string& name, const rapidjson::Value& type_json_value); + /// Parses a schema field, requiring explicit IDs on the field and all nested ROW fields. + /// + /// @param field_json_value The JSON value representing the field. + /// @return A Result containing the parsed DataField, or an error status if parsing fails. + static Result ParseDataField(const rapidjson::Value& field_json_value); + private: + /// Shares ID assignment across a type tree and rejects partially specified IDs. + class FieldIdAssigner { + public: + Result Assign(const std::optional& explicit_id); + + private: + int32_t next_generated_id_ = 0; + bool has_explicit_id_ = false; + }; + + /// Reuses the type tree's ID assigner; nullptr requires explicit IDs on all ROW fields. + static Result> ParseType(const std::string& name, + const rapidjson::Value& type_json_value, + FieldIdAssigner* field_id_assigner); + + static Result ParseDataField(const rapidjson::Value& field_json_value, + FieldIdAssigner* field_id_assigner); + static Result> ParseAtomicTypeField( const std::string& name, const rapidjson::Value& type_json_value); static Result> ParseComplexTypeField( - const std::string& name, const rapidjson::Value& type_json_value); + const std::string& name, const rapidjson::Value& type_json_value, + FieldIdAssigner* field_id_assigner); static Result> ParseArrayType( - const std::string& name, const rapidjson::Value& type_json_value, bool nullable); + const std::string& name, const rapidjson::Value& type_json_value, bool nullable, + FieldIdAssigner* field_id_assigner); static Result> ParseVectorType( - const std::string& name, const rapidjson::Value& type_json_value, bool nullable); + const std::string& name, const rapidjson::Value& type_json_value, bool nullable, + FieldIdAssigner* field_id_assigner); static Result> ParseMapType( - const std::string& name, const rapidjson::Value& type_json_value, bool nullable); + const std::string& name, const rapidjson::Value& type_json_value, bool nullable, + FieldIdAssigner* field_id_assigner); static Result> ParseRowType( - const std::string& name, const rapidjson::Value& type_json_value, bool nullable); + const std::string& name, const rapidjson::Value& type_json_value, bool nullable, + FieldIdAssigner* field_id_assigner); }; } // namespace paimon diff --git a/src/paimon/common/types/data_type_json_parser_test.cpp b/src/paimon/common/types/data_type_json_parser_test.cpp index e5dfbc21e..5049afc38 100644 --- a/src/paimon/common/types/data_type_json_parser_test.cpp +++ b/src/paimon/common/types/data_type_json_parser_test.cpp @@ -24,6 +24,7 @@ #include "gtest/gtest.h" #include "paimon/common/data/variant/variant_type_utils.h" +#include "paimon/common/types/data_field.h" #include "paimon/common/utils/checked_cast.h" #include "paimon/common/utils/date_time_utils.h" #include "paimon/status.h" @@ -34,6 +35,15 @@ namespace paimon::test { +namespace { + +void CheckFieldId(const std::shared_ptr& field, int32_t expected_id) { + ASSERT_OK_AND_ASSIGN(DataField data_field, DataField::ConvertArrowFieldToDataField(field)); + ASSERT_EQ(data_field.Id(), expected_id); +} + +} // namespace + TEST(DataTypeJsonParserTest, ParseTypeArrayTypeSuccess) { const std::string name = "array_field"; const char* json = R"({ @@ -127,6 +137,13 @@ TEST(DataTypeJsonParserTest, ParseTypeRowTypeSuccess) { "id" : 4, "name" : "sub4", "type" : "BYTES" + }, { + "id" : 7, + "name" : "sub7", + "type" : { + "type" : "ROW", + "fields" : [ { "id" : 9, "name" : "nested", "type" : "INT" } ] + } }]})"; rapidjson::Document doc; doc.Parse(json); @@ -134,6 +151,116 @@ TEST(DataTypeJsonParserTest, ParseTypeRowTypeSuccess) { ASSERT_OK_AND_ASSIGN(std::shared_ptr field, DataTypeJsonParser::ParseType(name, doc)); ASSERT_NE(field, nullptr); + ASSERT_EQ(field->type()->num_fields(), 3); + ASSERT_NO_FATAL_FAILURE(CheckFieldId(field->type()->field(0), 1)); + ASSERT_NO_FATAL_FAILURE(CheckFieldId(field->type()->field(1), 4)); + ASSERT_NO_FATAL_FAILURE(CheckFieldId(field->type()->field(2), 7)); + ASSERT_NO_FATAL_FAILURE(CheckFieldId(field->type()->field(2)->type()->field(0), 9)); +} + +TEST(DataTypeJsonParserTest, ParseTypeRowTypeGeneratesMissingFieldIds) { + const char* json = R"({ + "type" : "ROW", + "fields" : [ { + "name" : "payload", + "type" : { + "type" : "ROW", + "fields" : [ { + "name" : "age", + "type" : "INT" + }, { + "name" : "profile", + "type" : { + "type" : "ROW", + "fields" : [ { "name" : "city", "type" : "STRING" } ] + } + } ] + } + }, { + "name" : "tags", + "type" : { + "type" : "ARRAY", + "element" : { + "type" : "ROW", + "fields" : [ { "name" : "tag", "type" : "STRING" } ] + } + } + }, { + "name" : "props", + "type" : { + "type" : "MAP", + "key" : { + "type" : "ROW", + "fields" : [ { "name" : "code", "type" : "STRING" } ] + }, + "value" : { + "type" : "ROW", + "fields" : [ { "name" : "weight", "type" : "DOUBLE" } ] + } + } + } ]})"; + rapidjson::Document doc; + doc.Parse(json); + + for (int32_t attempt = 0; attempt < 2; ++attempt) { + SCOPED_TRACE(attempt); + ASSERT_OK_AND_ASSIGN(std::shared_ptr field, + DataTypeJsonParser::ParseType("row_field", doc)); + const std::shared_ptr& row = field->type(); + ASSERT_EQ(row->num_fields(), 3); + ASSERT_NO_FATAL_FAILURE(CheckFieldId(row->field(0), 0)); + const std::shared_ptr& payload = row->field(0)->type(); + ASSERT_EQ(payload->num_fields(), 2); + ASSERT_NO_FATAL_FAILURE(CheckFieldId(payload->field(0), 1)); + ASSERT_NO_FATAL_FAILURE(CheckFieldId(payload->field(1), 2)); + ASSERT_NO_FATAL_FAILURE(CheckFieldId(payload->field(1)->type()->field(0), 3)); + ASSERT_NO_FATAL_FAILURE(CheckFieldId(row->field(1), 4)); + const std::shared_ptr& element = row->field(1)->type()->field(0)->type(); + ASSERT_NO_FATAL_FAILURE(CheckFieldId(element->field(0), 5)); + ASSERT_NO_FATAL_FAILURE(CheckFieldId(row->field(2), 6)); + const std::shared_ptr& entries = row->field(2)->type()->field(0)->type(); + const std::shared_ptr& map_key = entries->field(0)->type(); + ASSERT_NO_FATAL_FAILURE(CheckFieldId(map_key->field(0), 7)); + const std::shared_ptr& map_value = entries->field(1)->type(); + ASSERT_NO_FATAL_FAILURE(CheckFieldId(map_value->field(0), 8)); + } +} + +TEST(DataTypeJsonParserTest, ParseTypeComplexTypeFailure) { + const std::vector> test_cases = { + {R"({"type":"ROW"})", "row data type must have fields"}, + {R"({"type":"ROW","fields":{}})", "row data type must have fields"}, + {R"({"type":"ROW","fields":[null]})", "data field must be an object"}, + {R"({"type":"ROW","fields":[{"name":"a","type":"INT"}, + {"id":1,"name":"b","type":"INT"}]})", + "Partial field id is not allowed."}, + {R"({"type":"ROW","fields":[{"id":0,"name":"a","type":"INT"}, + {"name":"b","type":"INT"}]})", + "Partial field id is not allowed."}, + {R"({"type":"ROW","fields":[{"id":0,"name":"a","type":{ + "type":"ROW","fields":[{"name":"b","type":"INT"}]}}]})", + "Partial field id is not allowed."}, + {R"({"type":"ROW","fields":[{"name":"a","type":{ + "type":"ROW","fields":[{"id":1,"name":"b","type":"INT"}]}}]})", + "Partial field id is not allowed."}, + {R"({"type":"ROW","fields":[{"name":"a","type":{"type":"ARRAY", + "element":{"type":"ROW","fields":[{"id":1,"name":"b","type":"INT"}]}}}]})", + "Partial field id is not allowed."}, + {R"({"type":"MAP", + "key":{"type":"ROW","fields":[{"id":0,"name":"a","type":"INT"}]}, + "value":{"type":"ROW","fields":[{"name":"b","type":"INT"}]}})", + "Partial field id is not allowed."}, + {R"({"type":"MAP", + "key":{"type":"ROW","fields":[{"name":"a","type":"INT"}]}, + "value":{"type":"ROW","fields":[{"id":1,"name":"b","type":"INT"}]}})", + "Partial field id is not allowed."}, + }; + for (const auto& [json, error_msg] : test_cases) { + SCOPED_TRACE(json); + rapidjson::Document doc; + doc.Parse(json); + ASSERT_NOK_WITH_MSG(DataTypeJsonParser::ParseType("row_field", doc), error_msg); + } } TEST(DataTypeJsonParserTest, ParseTypeAtomicTypeSuccess) { diff --git a/src/paimon/format/parquet/variant_parquet_test.cpp b/src/paimon/format/parquet/variant_parquet_test.cpp index 2c1bab1de..94c501f53 100644 --- a/src/paimon/format/parquet/variant_parquet_test.cpp +++ b/src/paimon/format/parquet/variant_parquet_test.cpp @@ -503,6 +503,24 @@ constexpr const char* kNestedShreddingSchema = R"({ } ] })"; +constexpr const char* kNestedShreddingSchemaWithoutIds = R"({ + "type": "ROW", + "fields": [ { + "name": "v", + "type": { + "type": "ROW", + "fields": [ + {"name": "age", "type": "INT"}, + {"name": "addr", "type": { + "type": "ROW", + "fields": [ {"name": "city", "type": "STRING"} ] + }}, + {"name": "tags", "type": {"type": "ARRAY", "element": "STRING"}} + ] + } + } ] +})"; + void CollectFieldIds(const ::parquet::schema::Node& node, const std::string& prefix, std::map* field_ids) { std::string path = prefix.empty() ? node.name() : prefix + "." + node.name(); @@ -648,12 +666,18 @@ TEST_F(VariantParquetTest, ShreddedWriteAndReadRoundTrip) { R"({"age": "not a number", "extra": [1, 2]})", "[\"top level array\"]", }; - for (const std::string mode : {"configured", "per-file", "adaptive"}) { + const std::map> expected_object_ids = { + {"configured", {{"age", 3}, {"addr", 4}, {"city", 5}, {"tags", 6}}}, + {"configured-without-ids", {{"age", 1}, {"addr", 2}, {"city", 3}, {"tags", 4}}}, + {"per-file", {{"age", 1}, {"addr", 0}, {"city", 0}, {"tags", 2}}}, + {"adaptive", {{"age", 1}, {"addr", 0}, {"city", 0}, {"tags", 2}}}}; + for (const auto& [mode, object_ids] : expected_object_ids) { SCOPED_TRACE(mode); - const bool configured = mode == "configured"; + const bool configured = mode == "configured" || mode == "configured-without-ids"; std::map option_map; if (configured) { - option_map[Options::VARIANT_SHREDDING_SCHEMA] = kNestedShreddingSchema; + option_map[Options::VARIANT_SHREDDING_SCHEMA] = + mode == "configured" ? kNestedShreddingSchema : kNestedShreddingSchemaWithoutIds; } else { option_map[Options::VARIANT_INFER_SHREDDING_SCHEMA] = "true"; option_map[Options::VARIANT_SHREDDING_INFERENCE_MODE] = mode; @@ -703,16 +727,16 @@ TEST_F(VariantParquetTest, ShreddedWriteAndReadRoundTrip) { {"v.metadata", 0}, {"v.value", 1}, {"v.typed_value", 2}, - {"v.typed_value.age", configured ? 3 : 1}, + {"v.typed_value.age", object_ids.at("age")}, {"v.typed_value.age.value", 0}, {"v.typed_value.age.typed_value", 1}, - {"v.typed_value.addr", configured ? 4 : 0}, + {"v.typed_value.addr", object_ids.at("addr")}, {"v.typed_value.addr.value", 0}, {"v.typed_value.addr.typed_value", 1}, - {"v.typed_value.addr.typed_value.city", configured ? 5 : 0}, + {"v.typed_value.addr.typed_value.city", object_ids.at("city")}, {"v.typed_value.addr.typed_value.city.value", 0}, {"v.typed_value.addr.typed_value.city.typed_value", 1}, - {"v.typed_value.tags", configured ? 6 : 2}, + {"v.typed_value.tags", object_ids.at("tags")}, {"v.typed_value.tags.value", 0}, {"v.typed_value.tags.typed_value", 1}, {"v.typed_value.tags.typed_value.list", -1},