From f58cf4f92a6f480ee8b0bd6b1a5dd18a45f0cfc2 Mon Sep 17 00:00:00 2001 From: MisterRaindrop <278811821@qq.com> Date: Wed, 2 Sep 2026 14:34:29 +0800 Subject: [PATCH 1/3] feat(hive): render Iceberg schemas as Hive DDL columns Creating an Iceberg table through HMS means rendering its schema as Hive DDL. Add `TypeToHiveString` for one `Type` and `SchemaToHiveColumns` for a whole `Schema`; nested struct / list / map render recursively. `timestamptz` follows iceberg-java and emits `timestamp with local time zone`, since engines reading through HMS take their column types from this DDL. Java's sub-Hive-3 fallback is not reproduced: there is no Hive-version probe here and the vendored IDL is Hive 4.0.1. Iceberg v3 types that no reference maps to a real Hive type (timestamp_ns, variant, geometry, geography, unknown) return kNotImplemented rather than guessing a spelling. --- src/iceberg/catalog/hive/CMakeLists.txt | 8 +- src/iceberg/catalog/hive/hive_schema.cc | 134 +++++++++++++++++ src/iceberg/catalog/hive/hive_schema.h | 93 ++++++++++++ src/iceberg/test/CMakeLists.txt | 2 +- src/iceberg/test/hive_schema_test.cc | 189 ++++++++++++++++++++++++ 5 files changed, 423 insertions(+), 3 deletions(-) create mode 100644 src/iceberg/catalog/hive/hive_schema.cc create mode 100644 src/iceberg/catalog/hive/hive_schema.h create mode 100644 src/iceberg/test/hive_schema_test.cc diff --git a/src/iceberg/catalog/hive/CMakeLists.txt b/src/iceberg/catalog/hive/CMakeLists.txt index 5ec191d389..a290fc3131 100644 --- a/src/iceberg/catalog/hive/CMakeLists.txt +++ b/src/iceberg/catalog/hive/CMakeLists.txt @@ -54,8 +54,12 @@ if(NOT TARGET thrift::thrift) "-DICEBERG_BUNDLE_THRIFT=OFF against a system Thrift install.") endif() -set(ICEBERG_HIVE_SOURCES hive_catalog.cc hive_catalog_properties.cc hms_client.cc - ${ICEBERG_HIVE_THRIFT_GEN_SOURCES}) +set(ICEBERG_HIVE_SOURCES + hive_catalog.cc + hive_catalog_properties.cc + hive_schema.cc + hms_client.cc + ${ICEBERG_HIVE_THRIFT_GEN_SOURCES}) set(ICEBERG_HIVE_STATIC_BUILD_INTERFACE_LIBS) set(ICEBERG_HIVE_SHARED_BUILD_INTERFACE_LIBS) diff --git a/src/iceberg/catalog/hive/hive_schema.cc b/src/iceberg/catalog/hive/hive_schema.cc new file mode 100644 index 0000000000..06fb387e8b --- /dev/null +++ b/src/iceberg/catalog/hive/hive_schema.cc @@ -0,0 +1,134 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/hive/hive_schema.h" + +#include +#include +#include + +#include "iceberg/schema.h" +#include "iceberg/schema_field.h" +#include "iceberg/type.h" +#include "iceberg/util/checked_cast.h" +#include "iceberg/util/macros.h" + +namespace iceberg::hive { + +namespace { + +// Hive doesn't have a precise counterpart to every Iceberg primitive +// type; the closest equivalent is chosen for each case below. Mapping +// matches iceberg-rust's `HiveSchemaBuilder::primitive`. +Result PrimitiveToHive(const Type& type) { + switch (type.type_id()) { + case TypeId::kBoolean: + return std::string("boolean"); + case TypeId::kInt: + return std::string("int"); + case TypeId::kLong: + return std::string("bigint"); + case TypeId::kFloat: + return std::string("float"); + case TypeId::kDouble: + return std::string("double"); + case TypeId::kDate: + return std::string("date"); + case TypeId::kTimestamp: + return std::string("timestamp"); + case TypeId::kTimestampTz: + // Hive's TIMESTAMPLOCALTZ, the spelling iceberg-java emits on + // Hive 3+. Engines reading through HMS must see the tz-adjusted + // type, otherwise they read an Iceberg timestamptz column as a + // naive timestamp. See the header for why the Hive 2 fallback is + // not reproduced. + return std::string("timestamp with local time zone"); + case TypeId::kTime: + case TypeId::kString: + case TypeId::kUuid: + return std::string("string"); + case TypeId::kBinary: + case TypeId::kFixed: + return std::string("binary"); + case TypeId::kDecimal: { + const auto& decimal = internal::checked_cast(type); + return std::format("decimal({},{})", decimal.precision(), decimal.scale()); + } + default: + return NotImplemented("Unhandled primitive Iceberg type id: {}", + static_cast(type.type_id())); + } +} + +Result StructToHive(const StructType& type) { + std::string out = "struct<"; + bool first = true; + for (const auto& field : type.fields()) { + if (!first) { + out += ","; + } + ICEBERG_ASSIGN_OR_RAISE(auto field_type, TypeToHiveString(*field.type())); + out += std::string(field.name()); + out += ":"; + out += field_type; + first = false; + } + out += ">"; + return out; +} + +Result ListToHive(const ListType& type) { + ICEBERG_ASSIGN_OR_RAISE(auto inner, TypeToHiveString(*type.element().type())); + return std::format("array<{}>", inner); +} + +Result MapToHive(const MapType& type) { + ICEBERG_ASSIGN_OR_RAISE(auto key_type, TypeToHiveString(*type.key().type())); + ICEBERG_ASSIGN_OR_RAISE(auto value_type, TypeToHiveString(*type.value().type())); + return std::format("map<{},{}>", key_type, value_type); +} + +} // namespace + +Result TypeToHiveString(const Type& type) { + switch (type.type_id()) { + case TypeId::kStruct: + return StructToHive(internal::checked_cast(type)); + case TypeId::kList: + return ListToHive(internal::checked_cast(type)); + case TypeId::kMap: + return MapToHive(internal::checked_cast(type)); + default: + return PrimitiveToHive(type); + } +} + +Result> SchemaToHiveColumns(const Schema& schema) { + std::vector columns; + columns.reserve(schema.fields().size()); + for (const auto& field : schema.fields()) { + ICEBERG_ASSIGN_OR_RAISE(auto type_string, TypeToHiveString(*field.type())); + columns.push_back(HiveColumn{.name = std::string(field.name()), + .type_string = std::move(type_string), + .comment = std::string(field.doc())}); + } + return columns; +} + +} // namespace iceberg::hive diff --git a/src/iceberg/catalog/hive/hive_schema.h b/src/iceberg/catalog/hive/hive_schema.h new file mode 100644 index 0000000000..4ae5d9413f --- /dev/null +++ b/src/iceberg/catalog/hive/hive_schema.h @@ -0,0 +1,93 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include +#include + +#include "iceberg/catalog/hive/iceberg_hive_export.h" +#include "iceberg/result.h" +#include "iceberg/type_fwd.h" + +/// \file iceberg/catalog/hive/hive_schema.h +/// \brief Convert an Iceberg `Schema` (or any nested `Type`) into the +/// Hive DDL representation that HMS expects on FieldSchema.type. +/// +/// The conversion is the C++ port of iceberg-rust's `HiveSchemaBuilder` +/// (`crates/catalog/hms/src/schema.rs`) and mirrors Java's +/// `HiveSchemaUtil` for the subset of types Hive can represent. + +namespace iceberg::hive { + +/// \brief A column entry for Hive's HMS table schema. +/// +/// HiveCatalog converts each top-level Iceberg `SchemaField` into one of +/// these, then HmsClient further wraps them into Thrift `FieldSchema` +/// records before passing to `create_table` / `alter_table`. Nested +/// types are flattened into the `type_string` field via DDL syntax +/// (e.g. `"struct"`, `"array"`, `"map"`). +struct ICEBERG_HIVE_EXPORT HiveColumn { + std::string name; + std::string type_string; + std::string comment; +}; + +/// \brief Render an Iceberg `Type` as a Hive DDL type string. +/// +/// Supported primitive mappings (matching iceberg-rust): +/// boolean -> boolean +/// int -> int +/// long -> bigint +/// float -> float +/// double -> double +/// date -> date +/// timestamp -> timestamp +/// timestamptz -> timestamp with local time zone +/// time / string / uuid -> string +/// binary / fixed -> binary +/// decimal(p, s) -> decimal(p,s) +/// struct<...> -> struct +/// list -> array +/// map -> map +/// +/// `timestamptz` follows iceberg-java, which emits Hive's +/// `timestamp with local time zone` (TIMESTAMPLOCALTZ). Engines that +/// read the table through HMS -- Hive, Spark, Trino -- take their +/// column types from this DDL, so downgrading to a naive `timestamp` +/// would silently misrepresent a tz-adjusted column to every one of +/// them. iceberg-rust instead rejects zoned timestamps outright, which +/// would make such tables uncreatable; that behaviour is not copied. +/// +/// Java picks the spelling at runtime and falls back to plain +/// `timestamp` below Hive 3, where TIMESTAMPLOCALTZ does not exist. +/// That fallback is not reproduced: iceberg-cpp has no Hive-version +/// probe, the vendored Metastore IDL is Hive 4.0.1, and Hive 2 has been +/// end-of-life since 2019. +ICEBERG_HIVE_EXPORT Result TypeToHiveString(const Type& type); + +/// \brief Convert each top-level field in `schema` to a `HiveColumn`. +/// +/// `optional` is intentionally not surfaced — HMS treats every column +/// as nullable. Iceberg `doc` is copied into `comment` so DESCRIBE +/// statements in Hive show the column documentation. +ICEBERG_HIVE_EXPORT Result> SchemaToHiveColumns( + const Schema& schema); + +} // namespace iceberg::hive diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index 46b1982c28..2ffd784527 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -362,7 +362,7 @@ if(ICEBERG_BUILD_HIVE) add_test(NAME ${test_name} COMMAND ${test_name}) endfunction() - add_hive_iceberg_test(hive_catalog_test SOURCES hms_client_test.cc) + add_hive_iceberg_test(hive_catalog_test SOURCES hive_schema_test.cc hms_client_test.cc) endif() if(ICEBERG_CXX20_COMPAT) diff --git a/src/iceberg/test/hive_schema_test.cc b/src/iceberg/test/hive_schema_test.cc new file mode 100644 index 0000000000..3b2eb9c374 --- /dev/null +++ b/src/iceberg/test/hive_schema_test.cc @@ -0,0 +1,189 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/hive/hive_schema.h" + +#include +#include + +#include + +#include "iceberg/schema.h" +#include "iceberg/schema_field.h" +#include "iceberg/type.h" + +namespace iceberg::hive { + +namespace { + +// Convenience: convert a primitive Iceberg type to its Hive DDL string, +// asserting success. +std::string ToHive(const std::shared_ptr& type) { + auto result = TypeToHiveString(*type); + EXPECT_TRUE(result.has_value()) + << "TypeToHiveString failed: " << (result ? "" : result.error().message); + return result.value_or(""); +} + +} // namespace + +TEST(TypeToHiveStringTest, PrimitivesMapToHiveScalarTypes) { + EXPECT_EQ(ToHive(boolean()), "boolean"); + EXPECT_EQ(ToHive(int32()), "int"); + EXPECT_EQ(ToHive(int64()), "bigint"); + EXPECT_EQ(ToHive(float32()), "float"); + EXPECT_EQ(ToHive(float64()), "double"); + EXPECT_EQ(ToHive(date()), "date"); + EXPECT_EQ(ToHive(timestamp()), "timestamp"); +} + +TEST(TypeToHiveStringTest, TimeStringUuidAllMapToString) { + EXPECT_EQ(ToHive(time()), "string"); + EXPECT_EQ(ToHive(string()), "string"); + EXPECT_EQ(ToHive(uuid()), "string"); +} + +TEST(TypeToHiveStringTest, BinaryAndFixedMapToBinary) { + EXPECT_EQ(ToHive(binary()), "binary"); + EXPECT_EQ(ToHive(fixed(16)), "binary"); +} + +TEST(TypeToHiveStringTest, DecimalIncludesPrecisionAndScale) { + EXPECT_EQ(ToHive(decimal(10, 2)), "decimal(10,2)"); + EXPECT_EQ(ToHive(decimal(38, 9)), "decimal(38,9)"); +} + +TEST(TypeToHiveStringTest, TimestampTzMapsToTimestampWithLocalTimeZone) { + // Hive's TIMESTAMPLOCALTZ, matching what iceberg-java emits on Hive 3+. + // A plain `timestamp` here would make every engine reading through HMS + // see a tz-adjusted column as naive. + EXPECT_EQ(ToHive(timestamp_tz()), "timestamp with local time zone"); +} + +TEST(TypeToHiveStringTest, StructFlattensFieldList) { + StructType struct_type( + {SchemaField(1, "a", int32(), true), SchemaField(2, "b", string(), true)}); + auto result = TypeToHiveString(struct_type); + ASSERT_TRUE(result.has_value()) << result.error().message; + EXPECT_EQ(*result, "struct"); +} + +TEST(TypeToHiveStringTest, ListWrapsElement) { + ListType list_type(1, int32(), true); + auto result = TypeToHiveString(list_type); + ASSERT_TRUE(result.has_value()) << result.error().message; + EXPECT_EQ(*result, "array"); +} + +TEST(TypeToHiveStringTest, MapEmitsKeyAndValue) { + MapType map_type(SchemaField(1, "key", string(), false), + SchemaField(2, "value", int32(), true)); + auto result = TypeToHiveString(map_type); + ASSERT_TRUE(result.has_value()) << result.error().message; + EXPECT_EQ(*result, "map"); +} + +TEST(TypeToHiveStringTest, NestedMapInsideMap) { + // map> + auto inner_map = std::make_shared(SchemaField(3, "key", string(), false), + SchemaField(4, "value", int32(), true)); + MapType outer_map(SchemaField(1, "key", string(), false), + SchemaField(2, "value", inner_map, true)); + auto result = TypeToHiveString(outer_map); + ASSERT_TRUE(result.has_value()) << result.error().message; + EXPECT_EQ(*result, "map>"); +} + +TEST(TypeToHiveStringTest, StructInsideListEmitsArrayOfStruct) { + auto struct_type = std::make_shared(std::vector{ + SchemaField(2, "name", string(), true), SchemaField(3, "age", int32(), true)}); + ListType list_type(SchemaField(1, "element", struct_type, true)); + auto result = TypeToHiveString(list_type); + ASSERT_TRUE(result.has_value()) << result.error().message; + EXPECT_EQ(*result, "array>"); +} + +TEST(TypeToHiveStringTest, NestedTimestampTzKeepsTheMultiWordTypeName) { + // The Hive type name contains spaces, and it stays spelled out inside a + // nested type. Java composes struct fields the same way -- via Hive's + // own TypeInfo round-trip -- so Hive's type parser accepts this form. + StructType struct_type( + {SchemaField(1, "a", int32(), true), SchemaField(2, "b", timestamp_tz(), true)}); + auto result = TypeToHiveString(struct_type); + ASSERT_TRUE(result.has_value()) << result.error().message; + EXPECT_EQ(*result, "struct"); +} + +TEST(SchemaToHiveColumnsTest, FlatSchemaProducesOneColumnPerField) { + Schema schema( + {SchemaField(1, "id", int64(), true), SchemaField(2, "name", string(), true)}, + /*schema_id=*/100); + + auto columns = SchemaToHiveColumns(schema); + ASSERT_TRUE(columns.has_value()) << columns.error().message; + ASSERT_EQ(columns->size(), 2); + + EXPECT_EQ((*columns)[0].name, "id"); + EXPECT_EQ((*columns)[0].type_string, "bigint"); + EXPECT_TRUE((*columns)[0].comment.empty()); + + EXPECT_EQ((*columns)[1].name, "name"); + EXPECT_EQ((*columns)[1].type_string, "string"); +} + +TEST(SchemaToHiveColumnsTest, NestedFieldRendersAsFlattenedTypeString) { + auto address = std::make_shared(std::vector{ + SchemaField(10, "street", string(), true), SchemaField(11, "zip", int32(), true)}); + Schema schema( + {SchemaField(1, "id", int64(), true), SchemaField(2, "address", address, true)}, + /*schema_id=*/200); + + auto columns = SchemaToHiveColumns(schema); + ASSERT_TRUE(columns.has_value()) << columns.error().message; + ASSERT_EQ(columns->size(), 2); + EXPECT_EQ((*columns)[1].name, "address"); + EXPECT_EQ((*columns)[1].type_string, "struct"); +} + +TEST(SchemaToHiveColumnsTest, FieldDocBecomesColumnComment) { + SchemaField id_field(1, "id", int64(), true, "Synthetic primary key."); + Schema schema({std::move(id_field)}, /*schema_id=*/1); + + auto columns = SchemaToHiveColumns(schema); + ASSERT_TRUE(columns.has_value()) << columns.error().message; + ASSERT_EQ(columns->size(), 1); + EXPECT_EQ((*columns)[0].comment, "Synthetic primary key."); +} + +TEST(SchemaToHiveColumnsTest, TimestampTzColumnIsTzAdjustedInTheDdl) { + // Distinguishing the two timestamp types is the whole point: a schema + // carrying both must not collapse them onto the same Hive type. + Schema schema({SchemaField(1, "ts", timestamp_tz(), true), + SchemaField(2, "naive", timestamp(), true)}, + /*schema_id=*/1); + auto columns = SchemaToHiveColumns(schema); + ASSERT_TRUE(columns.has_value()) << columns.error().message; + ASSERT_EQ(columns->size(), 2); + EXPECT_EQ((*columns)[0].name, "ts"); + EXPECT_EQ((*columns)[0].type_string, "timestamp with local time zone"); + EXPECT_EQ((*columns)[1].name, "naive"); + EXPECT_EQ((*columns)[1].type_string, "timestamp"); +} + +} // namespace iceberg::hive From d1165bf0458d3fb913b929144f0789599c8d6cea Mon Sep 17 00:00:00 2001 From: MisterRaindrop <278811821@qq.com> Date: Wed, 2 Sep 2026 14:34:29 +0800 Subject: [PATCH 2/3] feat(hive): add namespace and table conversion utilities HMS speaks `Database` and `Table`; Iceberg speaks `Namespace`, `TableIdentifier` and `Schema`. Add the conversion between them, plus the parameter keys (`metadata_location`, `table_type`, `EXTERNAL`) that iceberg-java and iceberg-rust use to recognise an Iceberg table, and the LazySimpleSerDe descriptor that keeps Spark and Trino reading it. The helpers produce plain structs rather than Thrift objects, so HmsClient stays the only translation unit that includes the generated bindings. A database owner is carried by `hive.metastore.database.owner` and `hive.metastore.database.owner-type`, the keys both references use; a bare `owner` is the table owner key. `owner-type` requires an owner and must name an HMS PrincipalType -- USER, GROUP or ROLE, compared case-insensitively. --- src/iceberg/catalog/hive/CMakeLists.txt | 1 + src/iceberg/catalog/hive/hive_utils.cc | 217 ++++++++++++++++++++++++ src/iceberg/catalog/hive/hive_utils.h | 177 +++++++++++++++++++ src/iceberg/test/CMakeLists.txt | 6 +- src/iceberg/test/hive_utils_test.cc | 194 +++++++++++++++++++++ 5 files changed, 594 insertions(+), 1 deletion(-) create mode 100644 src/iceberg/catalog/hive/hive_utils.cc create mode 100644 src/iceberg/catalog/hive/hive_utils.h create mode 100644 src/iceberg/test/hive_utils_test.cc diff --git a/src/iceberg/catalog/hive/CMakeLists.txt b/src/iceberg/catalog/hive/CMakeLists.txt index a290fc3131..8bcb835d6f 100644 --- a/src/iceberg/catalog/hive/CMakeLists.txt +++ b/src/iceberg/catalog/hive/CMakeLists.txt @@ -58,6 +58,7 @@ set(ICEBERG_HIVE_SOURCES hive_catalog.cc hive_catalog_properties.cc hive_schema.cc + hive_utils.cc hms_client.cc ${ICEBERG_HIVE_THRIFT_GEN_SOURCES}) diff --git a/src/iceberg/catalog/hive/hive_utils.cc b/src/iceberg/catalog/hive/hive_utils.cc new file mode 100644 index 0000000000..1b1fe18d10 --- /dev/null +++ b/src/iceberg/catalog/hive/hive_utils.cc @@ -0,0 +1,217 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/hive/hive_utils.h" + +#include +#include +#include +#include +#include +#include + +#include "iceberg/util/macros.h" +#include "iceberg/util/string_util.h" + +namespace iceberg::hive { + +namespace { + +// Keys lifted into dedicated `Database` fields, so they must not be +// duplicated into `parameters`. +constexpr std::array kDatabaseReservedKeys = { + kCommentProperty, kLocationProperty, kHmsDbOwnerProperty, kHmsDbOwnerTypeProperty}; + +// The table equivalent: `location` and `owner` have dedicated `Table` +// fields, and `comment` is not part of the Iceberg-on-HMS contract. +constexpr std::array kTableReservedKeys = { + kCommentProperty, kLocationProperty, kOwnerProperty}; + +// HMS `PrincipalType` names accepted for a database owner. +constexpr std::array kPrincipalTypes = {"USER", "GROUP", "ROLE"}; + +bool IsDatabaseReservedKey(std::string_view key) { + return std::ranges::find(kDatabaseReservedKeys, key) != kDatabaseReservedKeys.end(); +} + +bool IsTableReservedKey(std::string_view key) { + return std::ranges::find(kTableReservedKeys, key) != kTableReservedKeys.end(); +} + +std::string GetOrEmpty(const std::unordered_map& properties, + std::string_view key) { + auto it = properties.find(std::string(key)); + return it == properties.end() ? std::string() : it->second; +} + +std::string TrimTrailingSlash(std::string_view path) { + while (path.size() > 1 && path.back() == '/') { + path.remove_suffix(1); + } + return std::string(path); +} + +} // namespace + +Status ValidateNamespace(const Namespace& ns) { + if (ns.levels.size() != 1) { + return InvalidArgument( + "Hive Metastore only supports single-level namespaces; got {} level(s).", + ns.levels.size()); + } + if (ns.levels[0].empty()) { + return InvalidArgument("Hive namespace cannot have an empty name."); + } + return {}; +} + +Status ValidateOwnerSettings( + const std::unordered_map& properties) { + auto owner_type = properties.find(std::string(kHmsDbOwnerTypeProperty)); + if (owner_type == properties.end()) { + return {}; + } + if (!properties.contains(std::string(kHmsDbOwnerProperty))) { + return InvalidArgument("Hive namespace property '{}' requires '{}' to also be set.", + kHmsDbOwnerTypeProperty, kHmsDbOwnerProperty); + } + const bool known = std::ranges::any_of(kPrincipalTypes, [&](std::string_view name) { + return StringUtils::EqualsIgnoreCase(owner_type->second, name); + }); + if (!known) { + return InvalidArgument( + "Hive namespace property '{}' has value '{}'; expected one of " + "USER, GROUP or ROLE.", + kHmsDbOwnerTypeProperty, owner_type->second); + } + return {}; +} + +Result ConvertToHiveDatabase( + const Namespace& ns, const std::unordered_map& properties) { + ICEBERG_RETURN_UNEXPECTED(ValidateNamespace(ns)); + ICEBERG_RETURN_UNEXPECTED(ValidateOwnerSettings(properties)); + + HiveDatabase database; + database.name = ns.levels[0]; + database.description = GetOrEmpty(properties, kCommentProperty); + database.location_uri = GetOrEmpty(properties, kLocationProperty); + database.owner_name = GetOrEmpty(properties, kHmsDbOwnerProperty); + database.owner_type = GetOrEmpty(properties, kHmsDbOwnerTypeProperty); + + for (const auto& [key, value] : properties) { + if (!IsDatabaseReservedKey(key)) { + database.parameters.emplace(key, value); + } + } + return database; +} + +HiveNamespace ConvertFromHiveDatabase(const HiveDatabase& database) { + HiveNamespace result; + result.ns.levels = {database.name}; + result.properties = database.parameters; + if (!database.description.empty()) { + result.properties[std::string(kCommentProperty)] = database.description; + } + if (!database.location_uri.empty()) { + result.properties[std::string(kLocationProperty)] = database.location_uri; + } + if (!database.owner_name.empty()) { + result.properties[std::string(kHmsDbOwnerProperty)] = database.owner_name; + } + if (!database.owner_type.empty()) { + result.properties[std::string(kHmsDbOwnerTypeProperty)] = database.owner_type; + } + return result; +} + +Result ConvertToHiveTable( + const TableIdentifier& identifier, const std::vector& columns, + std::string_view metadata_location, std::string_view location, + const std::unordered_map& table_properties) { + ICEBERG_RETURN_UNEXPECTED(ValidateNamespace(identifier.ns)); + ICEBERG_RETURN_UNEXPECTED(identifier.Validate()); + + HiveTable table; + table.db_name = identifier.ns.levels[0]; + table.table_name = identifier.name; + table.owner = GetOrEmpty(table_properties, kOwnerProperty); + table.table_type = "EXTERNAL_TABLE"; + table.location = std::string(location); + table.columns = columns; + table.serde = std::string(kLazySimpleSerDe); + table.input_format = std::string(kFileInputFormat); + table.output_format = std::string(kFileOutputFormat); + + // Mandatory Iceberg-on-HMS marker parameters. + table.parameters.emplace(std::string(kMetadataLocationKey), metadata_location); + table.parameters.emplace(std::string(kTableTypeKey), std::string(kTableTypeIceberg)); + table.parameters.emplace(std::string(kExternalKey), std::string(kExternalTrue)); + + // Forward any user-supplied table properties that aren't reserved. + for (const auto& [key, value] : table_properties) { + if (!IsTableReservedKey(key) && !table.parameters.contains(key)) { + table.parameters.emplace(key, value); + } + } + return table; +} + +Result GetMetadataLocation( + const std::unordered_map& table_parameters) { + auto it = table_parameters.find(std::string(kMetadataLocationKey)); + if (it == table_parameters.end() || it->second.empty()) { + return NotFound("HMS table is missing '{}' parameter; not an Iceberg table.", + kMetadataLocationKey); + } + return it->second; +} + +Status ValidateIcebergTable( + const TableIdentifier& identifier, + const std::unordered_map& table_parameters) { + auto it = table_parameters.find(std::string(kTableTypeKey)); + if (it == table_parameters.end() || it->second.empty()) { + return NoSuchTable( + "HMS table {} has no '{}' parameter; refusing to treat it as an Iceberg table.", + identifier.ToString(), kTableTypeKey); + } + if (!StringUtils::EqualsIgnoreCase(it->second, kTableTypeIceberg)) { + return NoSuchTable("HMS table {} has '{}={}'; expected '{}' (case-insensitive).", + identifier.ToString(), kTableTypeKey, it->second, + kTableTypeIceberg); + } + return {}; +} + +std::string GetDefaultTableLocation(std::string_view warehouse, const Namespace& ns, + std::string_view table_name) { + std::string base = TrimTrailingSlash(warehouse); + std::string ns_part; + for (std::size_t i = 0; i < ns.levels.size(); ++i) { + if (i > 0) { + ns_part += "."; + } + ns_part += ns.levels[i]; + } + return std::format("{}/{}.db/{}", base, ns_part, table_name); +} + +} // namespace iceberg::hive diff --git a/src/iceberg/catalog/hive/hive_utils.h b/src/iceberg/catalog/hive/hive_utils.h new file mode 100644 index 0000000000..7868783937 --- /dev/null +++ b/src/iceberg/catalog/hive/hive_utils.h @@ -0,0 +1,177 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include +#include +#include +#include + +#include "iceberg/catalog/hive/hive_schema.h" +#include "iceberg/catalog/hive/iceberg_hive_export.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/hive/hive_utils.h +/// \brief Conversion helpers between Iceberg's catalog model +/// (Namespace, TableIdentifier, Schema) and the Hive Metastore +/// record shape that HmsClient expects on the wire. +/// +/// These helpers deliberately produce plain C++ structs rather than +/// Thrift `Database` / `Table` instances so that the public iceberg_hive +/// API stays free of Thrift includes. HmsClient is the only translation +/// unit that converts these structs into Thrift requests. + +namespace iceberg::hive { + +/// \brief Iceberg table parameter keys recognised by both +/// iceberg-java's HiveCatalog and iceberg-rust's HmsCatalog. +inline constexpr std::string_view kMetadataLocationKey = "metadata_location"; +inline constexpr std::string_view kTableTypeKey = "table_type"; +inline constexpr std::string_view kTableTypeIceberg = "ICEBERG"; +inline constexpr std::string_view kExternalKey = "EXTERNAL"; +inline constexpr std::string_view kExternalTrue = "TRUE"; + +/// \brief Iceberg namespace / table property keys used during conversion. +inline constexpr std::string_view kCommentProperty = "comment"; +inline constexpr std::string_view kLocationProperty = "location"; + +/// \brief HMS table owner property key. +inline constexpr std::string_view kOwnerProperty = "owner"; + +/// \brief HMS *database* owner property keys. +/// +/// Both iceberg-java's `HiveCatalog` and iceberg-rust's `HmsCatalog` +/// namespace these, so a bare `owner` set on a namespace stays an +/// ordinary Hive parameter and only the qualified key reaches the +/// `Database.ownerName` / `Database.ownerType` fields. +inline constexpr std::string_view kHmsDbOwnerProperty = "hive.metastore.database.owner"; +inline constexpr std::string_view kHmsDbOwnerTypeProperty = + "hive.metastore.database.owner-type"; + +/// \brief Hive storage descriptor values written into table parameters. +/// These match what Spark/Trino expect to find on Iceberg-backed +/// Hive tables, so cross-engine reads keep working. +inline constexpr std::string_view kLazySimpleSerDe = + "org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe"; +inline constexpr std::string_view kFileInputFormat = + "org.apache.hadoop.mapred.FileInputFormat"; +inline constexpr std::string_view kFileOutputFormat = + "org.apache.hadoop.mapred.FileOutputFormat"; + +/// \brief Plain mirror of the Hive Metastore `Database` record fields +/// that iceberg_hive needs to construct or read. +struct ICEBERG_HIVE_EXPORT HiveDatabase { + std::string name; + std::string description; + std::string location_uri; + std::unordered_map parameters; + std::string owner_name; + std::string owner_type; // "USER", "GROUP" or "ROLE"; empty means HMS-default +}; + +/// \brief Plain mirror of the Hive Metastore `Table` record fields +/// that iceberg_hive cares about. HmsClient maps it to Thrift. +struct ICEBERG_HIVE_EXPORT HiveTable { + std::string db_name; + std::string table_name; + std::string owner; + std::string table_type; // "EXTERNAL_TABLE" for Iceberg tables + std::string location; + std::vector columns; + std::unordered_map parameters; + std::string serde; + std::string input_format; + std::string output_format; +}; + +/// \brief Result of converting a `HiveDatabase` back to Iceberg's model. +struct ICEBERG_HIVE_EXPORT HiveNamespace { + Namespace ns; + std::unordered_map properties; +}; + +/// \brief Build the `HiveDatabase` that represents `ns` together with +/// `properties`. Reserved Iceberg property keys (`comment`, +/// `location`, `hive.metastore.database.owner`, +/// `hive.metastore.database.owner-type`) are lifted into dedicated +/// Hive fields; everything else becomes an entry in +/// `parameters`. +/// +/// Returns `kInvalidArgument` when the namespace has more than one +/// level (HMS only supports flat namespaces) or when the owner +/// settings are inconsistent (see `ValidateOwnerSettings`). +ICEBERG_HIVE_EXPORT Result ConvertToHiveDatabase( + const Namespace& ns, const std::unordered_map& properties); + +/// \brief Inverse of `ConvertToHiveDatabase`: split a `HiveDatabase` +/// into the Iceberg namespace plus the property map a caller +/// of `GetNamespaceProperties` would see. +ICEBERG_HIVE_EXPORT HiveNamespace ConvertFromHiveDatabase(const HiveDatabase& database); + +/// \brief Assemble the `HiveTable` describing an Iceberg table backed +/// by a metadata file at `metadata_location`. Sets the standard +/// Iceberg table parameters (`table_type=ICEBERG`, +/// `EXTERNAL=TRUE`, `metadata_location`) plus a `LazySimpleSerDe` +/// storage descriptor so Spark / Trino can read the table. +/// +/// `location` is the table's filesystem root (typically +/// `/.db/`); see `GetDefaultTableLocation`. +ICEBERG_HIVE_EXPORT Result ConvertToHiveTable( + const TableIdentifier& identifier, const std::vector& columns, + std::string_view metadata_location, std::string_view location, + const std::unordered_map& table_properties); + +/// \brief Extract the `metadata_location` parameter from a HMS Table +/// parameter map. Returns `kNotFound` if absent (the table is +/// not an Iceberg table). +ICEBERG_HIVE_EXPORT Result GetMetadataLocation( + const std::unordered_map& table_parameters); + +/// \brief Validate that a HMS Table parameter map represents an Iceberg +/// table by checking `table_type == "ICEBERG"` (case-insensitive). +/// Returns `kNoSuchTable` when the parameter is absent or holds +/// any other value, matching the guard Java's +/// `BaseMetastoreTableOperations.refresh()` applies before reading +/// `metadata_location`. +ICEBERG_HIVE_EXPORT Status ValidateIcebergTable( + const TableIdentifier& identifier, + const std::unordered_map& table_parameters); + +/// \brief Compute a default table location under `warehouse`. The +/// layout matches what iceberg-java and iceberg-rust pick: +/// `/.db/`. +ICEBERG_HIVE_EXPORT std::string GetDefaultTableLocation(std::string_view warehouse, + const Namespace& ns, + std::string_view table_name); + +/// \brief Enforce that `ns` is a single-level namespace (HMS only +/// supports flat databases). +ICEBERG_HIVE_EXPORT Status ValidateNamespace(const Namespace& ns); + +/// \brief Enforce the owner invariants shared with iceberg-java and +/// iceberg-rust: `hive.metastore.database.owner-type` requires +/// `hive.metastore.database.owner`, and its value must be one of +/// HMS's `PrincipalType` names -- USER, GROUP or ROLE, compared +/// case-insensitively. +ICEBERG_HIVE_EXPORT Status +ValidateOwnerSettings(const std::unordered_map& properties); + +} // namespace iceberg::hive diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index 2ffd784527..956a0cbd6b 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -362,7 +362,11 @@ if(ICEBERG_BUILD_HIVE) add_test(NAME ${test_name} COMMAND ${test_name}) endfunction() - add_hive_iceberg_test(hive_catalog_test SOURCES hive_schema_test.cc hms_client_test.cc) + add_hive_iceberg_test(hive_catalog_test + SOURCES + hive_schema_test.cc + hive_utils_test.cc + hms_client_test.cc) endif() if(ICEBERG_CXX20_COMPAT) diff --git a/src/iceberg/test/hive_utils_test.cc b/src/iceberg/test/hive_utils_test.cc new file mode 100644 index 0000000000..4d6aa9ed1b --- /dev/null +++ b/src/iceberg/test/hive_utils_test.cc @@ -0,0 +1,194 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/hive/hive_utils.h" + +#include + +namespace iceberg::hive { + +TEST(ValidateNamespaceTest, SingleLevelAccepted) { + EXPECT_TRUE(ValidateNamespace(Namespace{{"warehouse"}}).has_value()); +} + +TEST(ValidateNamespaceTest, MultiLevelRejected) { + auto status = ValidateNamespace(Namespace{{"a", "b"}}); + ASSERT_FALSE(status.has_value()); + EXPECT_EQ(status.error().kind, ErrorKind::kInvalidArgument); +} + +TEST(ValidateNamespaceTest, EmptyLevelRejected) { + auto status = ValidateNamespace(Namespace{{""}}); + ASSERT_FALSE(status.has_value()); + EXPECT_EQ(status.error().kind, ErrorKind::kInvalidArgument); +} + +TEST(ValidateOwnerSettingsTest, OwnerTypeRequiresOwner) { + auto status = ValidateOwnerSettings({{"hive.metastore.database.owner-type", "USER"}}); + ASSERT_FALSE(status.has_value()); + EXPECT_EQ(status.error().kind, ErrorKind::kInvalidArgument); +} + +TEST(ValidateOwnerSettingsTest, OwnerOrOwnerPlusTypeAreOk) { + EXPECT_TRUE( + ValidateOwnerSettings({{"hive.metastore.database.owner", "alice"}}).has_value()); + EXPECT_TRUE(ValidateOwnerSettings({{"hive.metastore.database.owner", "alice"}, + {"hive.metastore.database.owner-type", "role"}}) + .has_value()); + EXPECT_TRUE(ValidateOwnerSettings({}).has_value()); +} + +TEST(ValidateOwnerSettingsTest, UnknownOwnerTypeRejected) { + auto status = ValidateOwnerSettings({{"hive.metastore.database.owner", "alice"}, + {"hive.metastore.database.owner-type", "ADMIN"}}); + ASSERT_FALSE(status.has_value()); + EXPECT_EQ(status.error().kind, ErrorKind::kInvalidArgument); +} + +TEST(ValidateOwnerSettingsTest, BareOwnerKeysAreOrdinaryProperties) { + // Only the `hive.metastore.database.*` keys drive Database.ownerName / + // ownerType, matching iceberg-java and iceberg-rust. A bare `owner-type` + // is just a namespace property and needs no matching `owner`. + EXPECT_TRUE(ValidateOwnerSettings({{"owner-type", "nonsense"}}).has_value()); +} + +TEST(ConvertToHiveDatabaseTest, LiftsReservedKeysIntoDedicatedFields) { + auto db = ConvertToHiveDatabase(Namespace{{"warehouse"}}, + {{"comment", "primary db"}, + {"location", "s3://bucket/wh/"}, + {"hive.metastore.database.owner", "alice"}, + {"hive.metastore.database.owner-" + "type", + "USER"}, + {"custom-prop", "value"}}); + ASSERT_TRUE(db.has_value()) << db.error().message; + EXPECT_EQ(db->name, "warehouse"); + EXPECT_EQ(db->description, "primary db"); + EXPECT_EQ(db->location_uri, "s3://bucket/wh/"); + EXPECT_EQ(db->owner_name, "alice"); + EXPECT_EQ(db->owner_type, "USER"); + ASSERT_EQ(db->parameters.size(), 1); + EXPECT_EQ(db->parameters.at("custom-prop"), "value"); +} + +TEST(ConvertToHiveDatabaseTest, RejectsHierarchicalNamespace) { + auto db = ConvertToHiveDatabase(Namespace{{"a", "b"}}, {}); + ASSERT_FALSE(db.has_value()); + EXPECT_EQ(db.error().kind, ErrorKind::kInvalidArgument); +} + +TEST(ConvertFromHiveDatabaseTest, RoundTripWithCustomProperties) { + auto original = ConvertToHiveDatabase(Namespace{{"warehouse"}}, + {{"comment", "primary"}, + {"hive.metastore.database.owner", "alice"}, + {"team", "data"}}); + ASSERT_TRUE(original.has_value()) << original.error().message; + + const auto round_tripped = ConvertFromHiveDatabase(*original); + EXPECT_EQ(round_tripped.ns.levels, std::vector{"warehouse"}); + EXPECT_EQ(round_tripped.properties.at("comment"), "primary"); + EXPECT_EQ(round_tripped.properties.at("hive.metastore.database.owner"), "alice"); + EXPECT_EQ(round_tripped.properties.at("team"), "data"); + EXPECT_FALSE(round_tripped.properties.contains("hive.metastore.database.owner-type")); +} + +TEST(ConvertToHiveTableTest, SetsIcebergMarkerParametersAndStorageDescriptor) { + TableIdentifier ident{.ns = Namespace{{"warehouse"}}, .name = "orders"}; + const std::vector columns = {{.name = "id", .type_string = "bigint"}, + {.name = "amount", .type_string = "double"}}; + auto table = ConvertToHiveTable(ident, columns, + /*metadata_location=*/ + "s3://bucket/wh/warehouse.db/" + "orders/metadata/v1.metadata.json", + /*location=*/"s3://bucket/wh/warehouse.db/orders", + /*table_properties=*/{{"format-version", "2"}}); + ASSERT_TRUE(table.has_value()) << table.error().message; + EXPECT_EQ(table->db_name, "warehouse"); + EXPECT_EQ(table->table_name, "orders"); + EXPECT_EQ(table->table_type, "EXTERNAL_TABLE"); + EXPECT_EQ(table->location, "s3://bucket/wh/warehouse.db/orders"); + EXPECT_EQ(table->columns.size(), 2); + EXPECT_EQ(table->columns[0].name, "id"); + EXPECT_EQ(table->columns[0].type_string, "bigint"); + + EXPECT_EQ(table->parameters.at("metadata_location"), + "s3://bucket/wh/warehouse.db/orders/metadata/v1.metadata.json"); + EXPECT_EQ(table->parameters.at("table_type"), "ICEBERG"); + EXPECT_EQ(table->parameters.at("EXTERNAL"), "TRUE"); + EXPECT_EQ(table->parameters.at("format-version"), "2"); + + EXPECT_EQ(table->serde, "org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe"); + EXPECT_EQ(table->input_format, "org.apache.hadoop.mapred.FileInputFormat"); + EXPECT_EQ(table->output_format, "org.apache.hadoop.mapred.FileOutputFormat"); +} + +TEST(ConvertToHiveTableTest, ReservedTablePropertiesAreFiltered) { + TableIdentifier ident{.ns = Namespace{{"warehouse"}}, .name = "orders"}; + auto table = ConvertToHiveTable(ident, /*columns=*/{}, /*metadata_location=*/"loc", + /*location=*/"root", + {{"comment", "ignore me"}, {"owner", "bob"}}); + ASSERT_TRUE(table.has_value()) << table.error().message; + EXPECT_EQ(table->owner, "bob"); + EXPECT_FALSE(table->parameters.contains("comment")); + EXPECT_FALSE(table->parameters.contains("owner")); +} + +TEST(GetMetadataLocationTest, ExtractsKnownKey) { + auto loc = GetMetadataLocation({{"metadata_location", "s3://m.json"}}); + ASSERT_TRUE(loc.has_value()) << loc.error().message; + EXPECT_EQ(*loc, "s3://m.json"); +} + +TEST(GetMetadataLocationTest, AbsenceMapsToNotFound) { + auto loc = GetMetadataLocation({{"EXTERNAL", "TRUE"}}); + ASSERT_FALSE(loc.has_value()); + EXPECT_EQ(loc.error().kind, ErrorKind::kNotFound); +} + +TEST(ValidateIcebergTableTest, AcceptsCaseInsensitiveIcebergMarker) { + const TableIdentifier id{.ns = Namespace{{"sales"}}, .name = "orders"}; + EXPECT_TRUE(ValidateIcebergTable(id, {{"table_type", "ICEBERG"}}).has_value()); + EXPECT_TRUE(ValidateIcebergTable(id, {{"table_type", "iceberg"}}).has_value()); + EXPECT_TRUE(ValidateIcebergTable(id, {{"table_type", "Iceberg"}}).has_value()); +} + +TEST(ValidateIcebergTableTest, RejectsMissingMarker) { + const TableIdentifier id{.ns = Namespace{{"sales"}}, .name = "orders"}; + auto status = ValidateIcebergTable(id, {{"EXTERNAL", "TRUE"}}); + ASSERT_FALSE(status.has_value()); + EXPECT_EQ(status.error().kind, ErrorKind::kNoSuchTable); +} + +TEST(ValidateIcebergTableTest, RejectsForeignTableType) { + const TableIdentifier id{.ns = Namespace{{"sales"}}, .name = "orders"}; + auto status = + ValidateIcebergTable(id, {{"table_type", "MANAGED_TABLE"}, {"EXTERNAL", "TRUE"}}); + ASSERT_FALSE(status.has_value()); + EXPECT_EQ(status.error().kind, ErrorKind::kNoSuchTable); +} + +TEST(GetDefaultTableLocationTest, TrimsTrailingSlashAndAppendsDbAndTable) { + EXPECT_EQ( + GetDefaultTableLocation("s3://bucket/wh/", Namespace{{"warehouse"}}, "orders"), + "s3://bucket/wh/warehouse.db/orders"); + EXPECT_EQ(GetDefaultTableLocation("hdfs://nn/data", Namespace{{"sales"}}, "orders"), + "hdfs://nn/data/sales.db/orders"); +} + +} // namespace iceberg::hive From c5541b43e40c534f5087af8dd913b42decda55b9 Mon Sep 17 00:00:00 2001 From: Gang Wu Date: Fri, 9 Oct 2026 21:52:14 +0800 Subject: [PATCH 3/3] fix(hive): align conversions with Java --- .../IcebergThirdpartyToolchain.cmake | 5 +- src/iceberg/catalog/hive/CMakeLists.txt | 9 +- src/iceberg/catalog/hive/hive_schema.cc | 10 +-- src/iceberg/catalog/hive/hive_schema.h | 56 ++---------- src/iceberg/catalog/hive/hive_utils.cc | 43 +++++---- src/iceberg/catalog/hive/hive_utils.h | 88 +++++-------------- src/iceberg/cpp20_compatibility_internal.h | 2 + src/iceberg/test/hive_schema_test.cc | 40 ++++++--- src/iceberg/test/hive_utils_test.cc | 86 ++++++++++++++++-- 9 files changed, 165 insertions(+), 174 deletions(-) diff --git a/cmake_modules/IcebergThirdpartyToolchain.cmake b/cmake_modules/IcebergThirdpartyToolchain.cmake index 7631f0e0a0..9e0d3d3f5f 100644 --- a/cmake_modules/IcebergThirdpartyToolchain.cmake +++ b/cmake_modules/IcebergThirdpartyToolchain.cmake @@ -906,10 +906,7 @@ function(resolve_thrift_dependency) return() endif() if(ICEBERG_THRIFT_BUNDLED) - # Arrow's bundled build creates the Thrift C++ runtime as a `thrift` target - # scoped to its FetchContent directory, where iceberg_hive cannot see it. - # Promote it to a global `thrift::thrift` alias so iceberg_hive can link the - # generated Hive Metastore bindings against it. + # Expose bundled Thrift and its include paths to Hive. if(TARGET thrift AND NOT TARGET thrift::thrift) add_library(thrift::thrift INTERFACE IMPORTED GLOBAL) target_link_libraries(thrift::thrift INTERFACE thrift) diff --git a/src/iceberg/catalog/hive/CMakeLists.txt b/src/iceberg/catalog/hive/CMakeLists.txt index 8bcb835d6f..ff9f7a8aec 100644 --- a/src/iceberg/catalog/hive/CMakeLists.txt +++ b/src/iceberg/catalog/hive/CMakeLists.txt @@ -77,10 +77,11 @@ set(ICEBERG_HIVE_SHARED_INSTALL_INTERFACE_LIBS) # * System: downstream find_package(Iceberg) re-creates `thrift::thrift` via # find_dependency(ThriftAlt) (FindThriftAlt.cmake is shipped next to the # config), so it must be propagated. -list(APPEND ICEBERG_HIVE_STATIC_BUILD_INTERFACE_LIBS - "$,iceberg_static,iceberg_shared>" thrift::thrift) -list(APPEND ICEBERG_HIVE_SHARED_BUILD_INTERFACE_LIBS - "$,iceberg_shared,iceberg_static>" thrift::thrift) +# Prefer Thrift headers over any system Thrift found through core. +list(APPEND ICEBERG_HIVE_STATIC_BUILD_INTERFACE_LIBS thrift::thrift + "$,iceberg_static,iceberg_shared>") +list(APPEND ICEBERG_HIVE_SHARED_BUILD_INTERFACE_LIBS thrift::thrift + "$,iceberg_shared,iceberg_static>") list(APPEND ICEBERG_HIVE_STATIC_INSTALL_INTERFACE_LIBS "$,iceberg::iceberg_static,iceberg::iceberg_shared>" diff --git a/src/iceberg/catalog/hive/hive_schema.cc b/src/iceberg/catalog/hive/hive_schema.cc index 06fb387e8b..8311923a72 100644 --- a/src/iceberg/catalog/hive/hive_schema.cc +++ b/src/iceberg/catalog/hive/hive_schema.cc @@ -33,9 +33,6 @@ namespace iceberg::hive { namespace { -// Hive doesn't have a precise counterpart to every Iceberg primitive -// type; the closest equivalent is chosen for each case below. Mapping -// matches iceberg-rust's `HiveSchemaBuilder::primitive`. Result PrimitiveToHive(const Type& type) { switch (type.type_id()) { case TypeId::kBoolean: @@ -53,11 +50,6 @@ Result PrimitiveToHive(const Type& type) { case TypeId::kTimestamp: return std::string("timestamp"); case TypeId::kTimestampTz: - // Hive's TIMESTAMPLOCALTZ, the spelling iceberg-java emits on - // Hive 3+. Engines reading through HMS must see the tz-adjusted - // type, otherwise they read an Iceberg timestamptz column as a - // naive timestamp. See the header for why the Hive 2 fallback is - // not reproduced. return std::string("timestamp with local time zone"); case TypeId::kTime: case TypeId::kString: @@ -66,6 +58,8 @@ Result PrimitiveToHive(const Type& type) { case TypeId::kBinary: case TypeId::kFixed: return std::string("binary"); + case TypeId::kVariant: + return std::string("unknown"); case TypeId::kDecimal: { const auto& decimal = internal::checked_cast(type); return std::format("decimal({},{})", decimal.precision(), decimal.scale()); diff --git a/src/iceberg/catalog/hive/hive_schema.h b/src/iceberg/catalog/hive/hive_schema.h index 4ae5d9413f..8a093125de 100644 --- a/src/iceberg/catalog/hive/hive_schema.h +++ b/src/iceberg/catalog/hive/hive_schema.h @@ -27,66 +27,24 @@ #include "iceberg/type_fwd.h" /// \file iceberg/catalog/hive/hive_schema.h -/// \brief Convert an Iceberg `Schema` (or any nested `Type`) into the -/// Hive DDL representation that HMS expects on FieldSchema.type. -/// -/// The conversion is the C++ port of iceberg-rust's `HiveSchemaBuilder` -/// (`crates/catalog/hms/src/schema.rs`) and mirrors Java's -/// `HiveSchemaUtil` for the subset of types Hive can represent. +/// \brief Render Iceberg schemas as Hive type strings. namespace iceberg::hive { -/// \brief A column entry for Hive's HMS table schema. -/// -/// HiveCatalog converts each top-level Iceberg `SchemaField` into one of -/// these, then HmsClient further wraps them into Thrift `FieldSchema` -/// records before passing to `create_table` / `alter_table`. Nested -/// types are flattened into the `type_string` field via DDL syntax -/// (e.g. `"struct"`, `"array"`, `"map"`). +/// \brief Column metadata passed to HMS. struct ICEBERG_HIVE_EXPORT HiveColumn { std::string name; std::string type_string; std::string comment; }; -/// \brief Render an Iceberg `Type` as a Hive DDL type string. -/// -/// Supported primitive mappings (matching iceberg-rust): -/// boolean -> boolean -/// int -> int -/// long -> bigint -/// float -> float -/// double -> double -/// date -> date -/// timestamp -> timestamp -/// timestamptz -> timestamp with local time zone -/// time / string / uuid -> string -/// binary / fixed -> binary -/// decimal(p, s) -> decimal(p,s) -/// struct<...> -> struct -/// list -> array -/// map -> map -/// -/// `timestamptz` follows iceberg-java, which emits Hive's -/// `timestamp with local time zone` (TIMESTAMPLOCALTZ). Engines that -/// read the table through HMS -- Hive, Spark, Trino -- take their -/// column types from this DDL, so downgrading to a naive `timestamp` -/// would silently misrepresent a tz-adjusted column to every one of -/// them. iceberg-rust instead rejects zoned timestamps outright, which -/// would make such tables uncreatable; that behaviour is not copied. -/// -/// Java picks the spelling at runtime and falls back to plain -/// `timestamp` below Hive 3, where TIMESTAMPLOCALTZ does not exist. -/// That fallback is not reproduced: iceberg-cpp has no Hive-version -/// probe, the vendored Metastore IDL is Hive 4.0.1, and Hive 2 has been -/// end-of-life since 2019. +/// \brief Render a Hive type string, including nested types. +/// Follow Java: variant -> unknown, time/uuid -> string, fixed -> binary. +/// Unsupported types return kNotImplemented. Zoned timestamps require Hive 3+. ICEBERG_HIVE_EXPORT Result TypeToHiveString(const Type& type); -/// \brief Convert each top-level field in `schema` to a `HiveColumn`. -/// -/// `optional` is intentionally not surfaced — HMS treats every column -/// as nullable. Iceberg `doc` is copied into `comment` so DESCRIBE -/// statements in Hive show the column documentation. +/// \brief Convert top-level fields to HMS columns. Docs become comments. +/// Field IDs and requiredness are not represented in HMS. ICEBERG_HIVE_EXPORT Result> SchemaToHiveColumns( const Schema& schema); diff --git a/src/iceberg/catalog/hive/hive_utils.cc b/src/iceberg/catalog/hive/hive_utils.cc index 1b1fe18d10..6a7d573d7d 100644 --- a/src/iceberg/catalog/hive/hive_utils.cc +++ b/src/iceberg/catalog/hive/hive_utils.cc @@ -26,6 +26,7 @@ #include #include +#include "iceberg/table_properties.h" #include "iceberg/util/macros.h" #include "iceberg/util/string_util.h" @@ -33,15 +34,11 @@ namespace iceberg::hive { namespace { -// Keys lifted into dedicated `Database` fields, so they must not be -// duplicated into `parameters`. +// Keys lifted into dedicated Database fields. constexpr std::array kDatabaseReservedKeys = { kCommentProperty, kLocationProperty, kHmsDbOwnerProperty, kHmsDbOwnerTypeProperty}; -// The table equivalent: `location` and `owner` have dedicated `Table` -// fields, and `comment` is not part of the Iceberg-on-HMS contract. -constexpr std::array kTableReservedKeys = { - kCommentProperty, kLocationProperty, kOwnerProperty}; +constexpr std::string_view kExternalTablePurgeProperty = "external.table.purge"; // HMS `PrincipalType` names accepted for a database owner. constexpr std::array kPrincipalTypes = {"USER", "GROUP", "ROLE"}; @@ -50,10 +47,6 @@ bool IsDatabaseReservedKey(std::string_view key) { return std::ranges::find(kDatabaseReservedKeys, key) != kDatabaseReservedKeys.end(); } -bool IsTableReservedKey(std::string_view key) { - return std::ranges::find(kTableReservedKeys, key) != kTableReservedKeys.end(); -} - std::string GetOrEmpty(const std::unordered_map& properties, std::string_view key) { auto it = properties.find(std::string(key)); @@ -91,9 +84,8 @@ Status ValidateOwnerSettings( return InvalidArgument("Hive namespace property '{}' requires '{}' to also be set.", kHmsDbOwnerTypeProperty, kHmsDbOwnerProperty); } - const bool known = std::ranges::any_of(kPrincipalTypes, [&](std::string_view name) { - return StringUtils::EqualsIgnoreCase(owner_type->second, name); - }); + const bool known = + std::ranges::find(kPrincipalTypes, owner_type->second) != kPrincipalTypes.end(); if (!known) { return InvalidArgument( "Hive namespace property '{}' has value '{}'; expected one of " @@ -148,11 +140,14 @@ Result ConvertToHiveTable( const std::unordered_map& table_properties) { ICEBERG_RETURN_UNEXPECTED(ValidateNamespace(identifier.ns)); ICEBERG_RETURN_UNEXPECTED(identifier.Validate()); + if (metadata_location.empty()) { + return InvalidArgument("Hive table metadata location cannot be empty."); + } HiveTable table; table.db_name = identifier.ns.levels[0]; table.table_name = identifier.name; - table.owner = GetOrEmpty(table_properties, kOwnerProperty); + table.owner = GetOrEmpty(table_properties, kHmsTableOwnerProperty); table.table_type = "EXTERNAL_TABLE"; table.location = std::string(location); table.columns = columns; @@ -160,17 +155,19 @@ Result ConvertToHiveTable( table.input_format = std::string(kFileInputFormat); table.output_format = std::string(kFileOutputFormat); - // Mandatory Iceberg-on-HMS marker parameters. - table.parameters.emplace(std::string(kMetadataLocationKey), metadata_location); - table.parameters.emplace(std::string(kTableTypeKey), std::string(kTableTypeIceberg)); - table.parameters.emplace(std::string(kExternalKey), std::string(kExternalTrue)); - - // Forward any user-supplied table properties that aren't reserved. for (const auto& [key, value] : table_properties) { - if (!IsTableReservedKey(key) && !table.parameters.contains(key)) { - table.parameters.emplace(key, value); + if (StringUtils::EqualsIgnoreCase(key, kHmsTableOwnerProperty)) { + continue; } - } + const std::string hms_key = key == TableProperties::kGcEnabled.key() + ? std::string(kExternalTablePurgeProperty) + : key; + table.parameters.insert_or_assign(hms_key, value); + } + // The catalog markers take precedence over user properties. + table.parameters[std::string(kMetadataLocationKey)] = std::string(metadata_location); + table.parameters[std::string(kTableTypeKey)] = std::string(kTableTypeIceberg); + table.parameters[std::string(kExternalKey)] = std::string(kExternalTrue); return table; } diff --git a/src/iceberg/catalog/hive/hive_utils.h b/src/iceberg/catalog/hive/hive_utils.h index 7868783937..ad41af721e 100644 --- a/src/iceberg/catalog/hive/hive_utils.h +++ b/src/iceberg/catalog/hive/hive_utils.h @@ -30,45 +30,30 @@ #include "iceberg/table_identifier.h" /// \file iceberg/catalog/hive/hive_utils.h -/// \brief Conversion helpers between Iceberg's catalog model -/// (Namespace, TableIdentifier, Schema) and the Hive Metastore -/// record shape that HmsClient expects on the wire. -/// -/// These helpers deliberately produce plain C++ structs rather than -/// Thrift `Database` / `Table` instances so that the public iceberg_hive -/// API stays free of Thrift includes. HmsClient is the only translation -/// unit that converts these structs into Thrift requests. +/// \brief Convert Iceberg catalog objects to HMS records. namespace iceberg::hive { -/// \brief Iceberg table parameter keys recognised by both -/// iceberg-java's HiveCatalog and iceberg-rust's HmsCatalog. +/// \brief Iceberg-on-HMS parameter keys. inline constexpr std::string_view kMetadataLocationKey = "metadata_location"; inline constexpr std::string_view kTableTypeKey = "table_type"; inline constexpr std::string_view kTableTypeIceberg = "ICEBERG"; inline constexpr std::string_view kExternalKey = "EXTERNAL"; inline constexpr std::string_view kExternalTrue = "TRUE"; -/// \brief Iceberg namespace / table property keys used during conversion. +/// \brief Property keys lifted into HMS fields. inline constexpr std::string_view kCommentProperty = "comment"; inline constexpr std::string_view kLocationProperty = "location"; -/// \brief HMS table owner property key. -inline constexpr std::string_view kOwnerProperty = "owner"; +/// \brief HMS table owner key. +inline constexpr std::string_view kHmsTableOwnerProperty = "hive.metastore.table.owner"; -/// \brief HMS *database* owner property keys. -/// -/// Both iceberg-java's `HiveCatalog` and iceberg-rust's `HmsCatalog` -/// namespace these, so a bare `owner` set on a namespace stays an -/// ordinary Hive parameter and only the qualified key reaches the -/// `Database.ownerName` / `Database.ownerType` fields. +/// \brief HMS database owner keys. inline constexpr std::string_view kHmsDbOwnerProperty = "hive.metastore.database.owner"; inline constexpr std::string_view kHmsDbOwnerTypeProperty = "hive.metastore.database.owner-type"; -/// \brief Hive storage descriptor values written into table parameters. -/// These match what Spark/Trino expect to find on Iceberg-backed -/// Hive tables, so cross-engine reads keep working. +/// \brief Fallback storage descriptor classes. inline constexpr std::string_view kLazySimpleSerDe = "org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe"; inline constexpr std::string_view kFileInputFormat = @@ -76,8 +61,7 @@ inline constexpr std::string_view kFileInputFormat = inline constexpr std::string_view kFileOutputFormat = "org.apache.hadoop.mapred.FileOutputFormat"; -/// \brief Plain mirror of the Hive Metastore `Database` record fields -/// that iceberg_hive needs to construct or read. +/// \brief Subset of HMS Database used by HiveCatalog. struct ICEBERG_HIVE_EXPORT HiveDatabase { std::string name; std::string description; @@ -87,8 +71,7 @@ struct ICEBERG_HIVE_EXPORT HiveDatabase { std::string owner_type; // "USER", "GROUP" or "ROLE"; empty means HMS-default }; -/// \brief Plain mirror of the Hive Metastore `Table` record fields -/// that iceberg_hive cares about. HmsClient maps it to Thrift. +/// \brief Subset of HMS Table used by HiveCatalog. struct ICEBERG_HIVE_EXPORT HiveTable { std::string db_name; std::string table_name; @@ -102,75 +85,46 @@ struct ICEBERG_HIVE_EXPORT HiveTable { std::string output_format; }; -/// \brief Result of converting a `HiveDatabase` back to Iceberg's model. +/// \brief Namespace and properties read from HMS. struct ICEBERG_HIVE_EXPORT HiveNamespace { Namespace ns; std::unordered_map properties; }; -/// \brief Build the `HiveDatabase` that represents `ns` together with -/// `properties`. Reserved Iceberg property keys (`comment`, -/// `location`, `hive.metastore.database.owner`, -/// `hive.metastore.database.owner-type`) are lifted into dedicated -/// Hive fields; everything else becomes an entry in -/// `parameters`. -/// -/// Returns `kInvalidArgument` when the namespace has more than one -/// level (HMS only supports flat namespaces) or when the owner -/// settings are inconsistent (see `ValidateOwnerSettings`). +/// \brief Lift reserved properties into HMS fields. Caller resolves defaults. ICEBERG_HIVE_EXPORT Result ConvertToHiveDatabase( const Namespace& ns, const std::unordered_map& properties); -/// \brief Inverse of `ConvertToHiveDatabase`: split a `HiveDatabase` -/// into the Iceberg namespace plus the property map a caller -/// of `GetNamespaceProperties` would see. +/// \brief Extract namespace properties from an HMS database. ICEBERG_HIVE_EXPORT HiveNamespace ConvertFromHiveDatabase(const HiveDatabase& database); -/// \brief Assemble the `HiveTable` describing an Iceberg table backed -/// by a metadata file at `metadata_location`. Sets the standard -/// Iceberg table parameters (`table_type=ICEBERG`, -/// `EXTERNAL=TRUE`, `metadata_location`) plus a `LazySimpleSerDe` -/// storage descriptor so Spark / Trino can read the table. -/// -/// `location` is the table's filesystem root (typically -/// `/.db/
`); see `GetDefaultTableLocation`. +/// \brief Build an external Iceberg table with a LazySimpleSerDe descriptor. +/// Translate gc.enabled to external.table.purge. Catalog markers override user +/// properties. metadata_location must be non-empty; caller resolves the owner. ICEBERG_HIVE_EXPORT Result ConvertToHiveTable( const TableIdentifier& identifier, const std::vector& columns, std::string_view metadata_location, std::string_view location, const std::unordered_map& table_properties); -/// \brief Extract the `metadata_location` parameter from a HMS Table -/// parameter map. Returns `kNotFound` if absent (the table is -/// not an Iceberg table). +/// \brief Return metadata_location, or kNotFound if absent or empty. ICEBERG_HIVE_EXPORT Result GetMetadataLocation( const std::unordered_map& table_parameters); -/// \brief Validate that a HMS Table parameter map represents an Iceberg -/// table by checking `table_type == "ICEBERG"` (case-insensitive). -/// Returns `kNoSuchTable` when the parameter is absent or holds -/// any other value, matching the guard Java's -/// `BaseMetastoreTableOperations.refresh()` applies before reading -/// `metadata_location`. +/// \brief Require table_type=ICEBERG (case-insensitive). ICEBERG_HIVE_EXPORT Status ValidateIcebergTable( const TableIdentifier& identifier, const std::unordered_map& table_parameters); -/// \brief Compute a default table location under `warehouse`. The -/// layout matches what iceberg-java and iceberg-rust pick: -/// `/.db/`. +/// \brief Return /.db/. ICEBERG_HIVE_EXPORT std::string GetDefaultTableLocation(std::string_view warehouse, const Namespace& ns, std::string_view table_name); -/// \brief Enforce that `ns` is a single-level namespace (HMS only -/// supports flat databases). +/// \brief Require a single-level namespace. ICEBERG_HIVE_EXPORT Status ValidateNamespace(const Namespace& ns); -/// \brief Enforce the owner invariants shared with iceberg-java and -/// iceberg-rust: `hive.metastore.database.owner-type` requires -/// `hive.metastore.database.owner`, and its value must be one of -/// HMS's `PrincipalType` names -- USER, GROUP or ROLE, compared -/// case-insensitively. +/// \brief Require owner when owner-type is set. +/// owner-type must be USER, GROUP or ROLE. ICEBERG_HIVE_EXPORT Status ValidateOwnerSettings(const std::unordered_map& properties); diff --git a/src/iceberg/cpp20_compatibility_internal.h b/src/iceberg/cpp20_compatibility_internal.h index 71a5ccce72..5c54b5441b 100644 --- a/src/iceberg/cpp20_compatibility_internal.h +++ b/src/iceberg/cpp20_compatibility_internal.h @@ -213,6 +213,8 @@ #ifdef ICEBERG_PUBLIC_HEADERS_HIVE # include "iceberg/catalog/hive/hive_catalog.h" # include "iceberg/catalog/hive/hive_catalog_properties.h" +# include "iceberg/catalog/hive/hive_schema.h" +# include "iceberg/catalog/hive/hive_utils.h" # include "iceberg/catalog/hive/hms_client.h" # include "iceberg/catalog/hive/iceberg_hive_export.h" #endif // ICEBERG_PUBLIC_HEADERS_HIVE diff --git a/src/iceberg/test/hive_schema_test.cc b/src/iceberg/test/hive_schema_test.cc index 3b2eb9c374..3eb88a304b 100644 --- a/src/iceberg/test/hive_schema_test.cc +++ b/src/iceberg/test/hive_schema_test.cc @@ -32,8 +32,7 @@ namespace iceberg::hive { namespace { -// Convenience: convert a primitive Iceberg type to its Hive DDL string, -// asserting success. +// Assert successful conversion. std::string ToHive(const std::shared_ptr& type) { auto result = TypeToHiveString(*type); EXPECT_TRUE(result.has_value()) @@ -64,15 +63,40 @@ TEST(TypeToHiveStringTest, BinaryAndFixedMapToBinary) { EXPECT_EQ(ToHive(fixed(16)), "binary"); } +TEST(SchemaToHiveColumnsTest, VariantUsesHiveMetadataPlaceholder) { + Schema schema({SchemaField(1, "payload", variant(), true)}); + auto columns = SchemaToHiveColumns(schema); + ASSERT_TRUE(columns.has_value()) << columns.error().message; + ASSERT_EQ(columns->size(), 1); + EXPECT_EQ(columns->front().name, "payload"); + EXPECT_EQ(columns->front().type_string, "unknown"); +} + +TEST(TypeToHiveStringTest, UnsupportedTypesReturnNotImplemented) { + const std::vector> types = {timestamp_ns(), timestamptz_ns(), + unknown(), geometry(), geography()}; + for (const auto& type : types) { + SCOPED_TRACE(type->ToString()); + auto result = TypeToHiveString(*type); + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(result.error().kind, ErrorKind::kNotImplemented); + } +} + +TEST(SchemaToHiveColumnsTest, PropagatesUnsupportedNestedType) { + auto list_type = std::make_shared(1, timestamp_ns(), true); + Schema schema({SchemaField(2, "values", list_type, true)}); + auto columns = SchemaToHiveColumns(schema); + ASSERT_FALSE(columns.has_value()); + EXPECT_EQ(columns.error().kind, ErrorKind::kNotImplemented); +} + TEST(TypeToHiveStringTest, DecimalIncludesPrecisionAndScale) { EXPECT_EQ(ToHive(decimal(10, 2)), "decimal(10,2)"); EXPECT_EQ(ToHive(decimal(38, 9)), "decimal(38,9)"); } TEST(TypeToHiveStringTest, TimestampTzMapsToTimestampWithLocalTimeZone) { - // Hive's TIMESTAMPLOCALTZ, matching what iceberg-java emits on Hive 3+. - // A plain `timestamp` here would make every engine reading through HMS - // see a tz-adjusted column as naive. EXPECT_EQ(ToHive(timestamp_tz()), "timestamp with local time zone"); } @@ -120,9 +144,6 @@ TEST(TypeToHiveStringTest, StructInsideListEmitsArrayOfStruct) { } TEST(TypeToHiveStringTest, NestedTimestampTzKeepsTheMultiWordTypeName) { - // The Hive type name contains spaces, and it stays spelled out inside a - // nested type. Java composes struct fields the same way -- via Hive's - // own TypeInfo round-trip -- so Hive's type parser accepts this form. StructType struct_type( {SchemaField(1, "a", int32(), true), SchemaField(2, "b", timestamp_tz(), true)}); auto result = TypeToHiveString(struct_type); @@ -172,8 +193,7 @@ TEST(SchemaToHiveColumnsTest, FieldDocBecomesColumnComment) { } TEST(SchemaToHiveColumnsTest, TimestampTzColumnIsTzAdjustedInTheDdl) { - // Distinguishing the two timestamp types is the whole point: a schema - // carrying both must not collapse them onto the same Hive type. + // Both timestamp types must stay distinct. Schema schema({SchemaField(1, "ts", timestamp_tz(), true), SchemaField(2, "naive", timestamp(), true)}, /*schema_id=*/1); diff --git a/src/iceberg/test/hive_utils_test.cc b/src/iceberg/test/hive_utils_test.cc index 4d6aa9ed1b..ba34808a42 100644 --- a/src/iceberg/test/hive_utils_test.cc +++ b/src/iceberg/test/hive_utils_test.cc @@ -49,7 +49,7 @@ TEST(ValidateOwnerSettingsTest, OwnerOrOwnerPlusTypeAreOk) { EXPECT_TRUE( ValidateOwnerSettings({{"hive.metastore.database.owner", "alice"}}).has_value()); EXPECT_TRUE(ValidateOwnerSettings({{"hive.metastore.database.owner", "alice"}, - {"hive.metastore.database.owner-type", "role"}}) + {"hive.metastore.database.owner-type", "ROLE"}}) .has_value()); EXPECT_TRUE(ValidateOwnerSettings({}).has_value()); } @@ -62,9 +62,6 @@ TEST(ValidateOwnerSettingsTest, UnknownOwnerTypeRejected) { } TEST(ValidateOwnerSettingsTest, BareOwnerKeysAreOrdinaryProperties) { - // Only the `hive.metastore.database.*` keys drive Database.ownerName / - // ownerType, matching iceberg-java and iceberg-rust. A bare `owner-type` - // is just a namespace property and needs no matching `owner`. EXPECT_TRUE(ValidateOwnerSettings({{"owner-type", "nonsense"}}).has_value()); } @@ -93,6 +90,14 @@ TEST(ConvertToHiveDatabaseTest, RejectsHierarchicalNamespace) { EXPECT_EQ(db.error().kind, ErrorKind::kInvalidArgument); } +TEST(ConvertToHiveDatabaseTest, RejectsNonCanonicalOwnerType) { + auto db = ConvertToHiveDatabase(Namespace{{"warehouse"}}, + {{"hive.metastore.database.owner", "alice"}, + {"hive.metastore.database.owner-type", "role"}}); + ASSERT_FALSE(db.has_value()); + EXPECT_EQ(db.error().kind, ErrorKind::kInvalidArgument); +} + TEST(ConvertFromHiveDatabaseTest, RoundTripWithCustomProperties) { auto original = ConvertToHiveDatabase(Namespace{{"warehouse"}}, {{"comment", "primary"}, @@ -132,21 +137,78 @@ TEST(ConvertToHiveTableTest, SetsIcebergMarkerParametersAndStorageDescriptor) { EXPECT_EQ(table->parameters.at("table_type"), "ICEBERG"); EXPECT_EQ(table->parameters.at("EXTERNAL"), "TRUE"); EXPECT_EQ(table->parameters.at("format-version"), "2"); + EXPECT_FALSE(table->parameters.contains("external.table.purge")); EXPECT_EQ(table->serde, "org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe"); EXPECT_EQ(table->input_format, "org.apache.hadoop.mapred.FileInputFormat"); EXPECT_EQ(table->output_format, "org.apache.hadoop.mapred.FileOutputFormat"); } -TEST(ConvertToHiveTableTest, ReservedTablePropertiesAreFiltered) { +TEST(ConvertToHiveTableTest, UsesQualifiedOwnerAndPreservesCommentAndBareOwner) { TableIdentifier ident{.ns = Namespace{{"warehouse"}}, .name = "orders"}; auto table = ConvertToHiveTable(ident, /*columns=*/{}, /*metadata_location=*/"loc", /*location=*/"root", - {{"comment", "ignore me"}, {"owner", "bob"}}); + {{"comment", "table description"}, + {"owner", "bob"}, + {"hive.metastore.table.owner", "alice"}, + {"location", "other root"}}); ASSERT_TRUE(table.has_value()) << table.error().message; - EXPECT_EQ(table->owner, "bob"); - EXPECT_FALSE(table->parameters.contains("comment")); - EXPECT_FALSE(table->parameters.contains("owner")); + EXPECT_EQ(table->owner, "alice"); + EXPECT_EQ(table->location, "root"); + EXPECT_FALSE(table->parameters.contains("hive.metastore.table.owner")); + EXPECT_EQ(table->parameters.at("location"), "other root"); + EXPECT_EQ(table->parameters.at("comment"), "table description"); + EXPECT_EQ(table->parameters.at("owner"), "bob"); +} + +TEST(ConvertToHiveTableTest, TranslatesGcEnabled) { + TableIdentifier ident{.ns = Namespace{{"warehouse"}}, .name = "orders"}; + for (const std::string value : {"true", "false"}) { + SCOPED_TRACE(value); + auto table = ConvertToHiveTable(ident, {}, "loc", "root", {{"gc.enabled", value}}); + ASSERT_TRUE(table.has_value()) << table.error().message; + EXPECT_EQ(table->parameters.at("external.table.purge"), value); + EXPECT_FALSE(table->parameters.contains("gc.enabled")); + } +} + +TEST(ConvertToHiveTableTest, ConflictingPurgePropertiesFollowIterationOrder) { + TableIdentifier ident{.ns = Namespace{{"warehouse"}}, .name = "orders"}; + const std::unordered_map properties = { + {"gc.enabled", "false"}, {"external.table.purge", "true"}}; + std::string expected; + for (const auto& [key, value] : properties) { + expected = value; + } + auto table = ConvertToHiveTable(ident, {}, "loc", "root", properties); + ASSERT_TRUE(table.has_value()) << table.error().message; + EXPECT_EQ(table->parameters.at("external.table.purge"), expected); +} + +TEST(ConvertToHiveTableTest, PreservesExplicitPurgePropertyWithoutGcEnabled) { + TableIdentifier ident{.ns = Namespace{{"warehouse"}}, .name = "orders"}; + auto table = + ConvertToHiveTable(ident, {}, "loc", "root", {{"external.table.purge", "true"}}); + ASSERT_TRUE(table.has_value()) << table.error().message; + EXPECT_EQ(table->parameters.at("external.table.purge"), "true"); +} + +TEST(ConvertToHiveTableTest, RejectsEmptyMetadataLocation) { + TableIdentifier ident{.ns = Namespace{{"warehouse"}}, .name = "orders"}; + auto table = ConvertToHiveTable(ident, {}, "", "root", {}); + ASSERT_FALSE(table.has_value()); + EXPECT_EQ(table.error().kind, ErrorKind::kInvalidArgument); +} + +TEST(ConvertToHiveTableTest, ProtectsIcebergMarkerParameters) { + TableIdentifier ident{.ns = Namespace{{"warehouse"}}, .name = "orders"}; + auto table = ConvertToHiveTable( + ident, {}, "loc", "root", + {{"metadata_location", "other"}, {"table_type", "OTHER"}, {"EXTERNAL", "FALSE"}}); + ASSERT_TRUE(table.has_value()) << table.error().message; + EXPECT_EQ(table->parameters.at("metadata_location"), "loc"); + EXPECT_EQ(table->parameters.at("table_type"), "ICEBERG"); + EXPECT_EQ(table->parameters.at("EXTERNAL"), "TRUE"); } TEST(GetMetadataLocationTest, ExtractsKnownKey) { @@ -161,6 +223,12 @@ TEST(GetMetadataLocationTest, AbsenceMapsToNotFound) { EXPECT_EQ(loc.error().kind, ErrorKind::kNotFound); } +TEST(GetMetadataLocationTest, EmptyLocationMapsToNotFound) { + auto loc = GetMetadataLocation({{"metadata_location", ""}}); + ASSERT_FALSE(loc.has_value()); + EXPECT_EQ(loc.error().kind, ErrorKind::kNotFound); +} + TEST(ValidateIcebergTableTest, AcceptsCaseInsensitiveIcebergMarker) { const TableIdentifier id{.ns = Namespace{{"sales"}}, .name = "orders"}; EXPECT_TRUE(ValidateIcebergTable(id, {{"table_type", "ICEBERG"}}).has_value());