wgtmac commented on code in PR #257: URL: https://github.com/apache/iceberg-cpp/pull/257#discussion_r2437994016
########## src/iceberg/table_update.h: ########## @@ -0,0 +1,356 @@ +/* + * 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 + +/// \file iceberg/table_update.h +/// Table metadata update operations for Iceberg tables. + +#include <memory> +#include <optional> +#include <string> +#include <unordered_map> +#include <vector> + +#include "iceberg/iceberg_export.h" +#include "iceberg/snapshot.h" +#include "iceberg/type_fwd.h" + +namespace iceberg { + +/// \brief Base class for metadata update operations +/// +/// Represents a change to table metadata. Each concrete subclass +/// represents a specific type of update operation. +class ICEBERG_EXPORT TableUpdate { + public: + virtual ~TableUpdate() = default; + + /// \brief Apply this update to a TableMetadataBuilder + /// + /// This method modifies the builder by applying the update operation + /// it represents. Each subclass implements this to apply its specific + /// type of update. + /// + /// \param builder The builder to apply this update to + virtual void ApplyTo(TableMetadataBuilder& builder) const = 0; + + /// \brief Generate update requirements for this metadata update + /// + /// This method generates the appropriate UpdateRequirement instances + /// that must be validated before this update can be applied. The context + /// provides information about the base metadata and operation mode. + /// + /// \param context The context containing base metadata and operation state + /// \return Status indicating success or failure with error details + virtual Status GenerateRequirements(MetadataUpdateContext& context) const = 0; +}; + +/// \brief Represents an assignment of a UUID to the table +class ICEBERG_EXPORT AssignUUID : public TableUpdate { Review Comment: Will it cause name conflicts once we add `class AssignUUID : public ViewUpdate`? Or do we want to modify it as `class AssignUUID : public TableUpdate, public ViewUpdate` ? If latter, we may need to rename the file to `metadata_update.h/cc`? ########## src/iceberg/table_requirements.h: ########## @@ -0,0 +1,118 @@ +/* + * 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 + +/// \file iceberg/update_requirements.h Review Comment: The comment looks inconsistent now. ########## src/iceberg/CMakeLists.txt: ########## @@ -47,7 +47,10 @@ set(ICEBERG_SOURCES table.cc table_metadata.cc table_properties.cc + table_requirement.cc Review Comment: @gty404 Please add new header and source files to the corresponding `meson.build` file. ########## src/iceberg/util/string_util.h: ########## @@ -44,6 +44,11 @@ class ICEBERG_EXPORT StringUtils { [](char c) { return std::toupper(c); }); // NOLINT return input; } + + static bool EqualsIgnoreCase(const std::string& a, const std::string& b) { Review Comment: +1 ########## src/iceberg/table_requirements.h: ########## @@ -0,0 +1,118 @@ +/* + * 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 + +/// \file iceberg/update_requirements.h +/// Factory for generating update requirements from metadata updates. +/// +/// This utility class generates the appropriate UpdateRequirement instances +/// based on a list of MetadataUpdate operations. The requirements are used +/// for optimistic concurrency control when committing table changes. + +#include <memory> +#include <vector> + +#include "iceberg/iceberg_export.h" +#include "iceberg/table_requirement.h" +#include "iceberg/type_fwd.h" + +namespace iceberg { + +/// \brief Context for generating update requirements +/// +/// This context is passed to each MetadataUpdate's GenerateRequirements method +/// and maintains state about what requirements have already been added to avoid +/// duplicates. +class ICEBERG_EXPORT MetadataUpdateContext { Review Comment: ```suggestion class ICEBERG_EXPORT TableUpdateContext { ``` Also fix the comments after renaming ########## src/iceberg/table_metadata.cc: ########## @@ -196,4 +197,185 @@ Status TableMetadataUtil::Write(FileIO& io, const std::string& location, return io.WriteFile(location, json_string); } +// TableMetadataBuilder implementation + +struct TableMetadataBuilder::Impl {}; + +TableMetadataBuilder::TableMetadataBuilder(int8_t format_version) + : impl_(std::make_unique<Impl>()) {} + +TableMetadataBuilder::TableMetadataBuilder(const TableMetadata* base) + : impl_(std::make_unique<Impl>()) {} + +TableMetadataBuilder::~TableMetadataBuilder() = default; + +TableMetadataBuilder::TableMetadataBuilder(TableMetadataBuilder&&) noexcept = default; + +TableMetadataBuilder& TableMetadataBuilder::operator=(TableMetadataBuilder&&) noexcept = + default; + +std::unique_ptr<TableMetadataBuilder> TableMetadataBuilder::BuildFromEmpty( + int8_t format_version) { + return std::unique_ptr<TableMetadataBuilder>( + new TableMetadataBuilder(format_version)); // NOLINT +} + +std::unique_ptr<TableMetadataBuilder> TableMetadataBuilder::BuildFrom( + const TableMetadata* base) { + return std::unique_ptr<TableMetadataBuilder>(new TableMetadataBuilder(base)); // NOLINT +} + +TableMetadataBuilder& TableMetadataBuilder::SetMetadataLocation( + std::string_view metadata_location) { + return *this; Review Comment: We should throw for unimplemented functions. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
