SteNicholas commented on code in PR #222:
URL: https://github.com/apache/paimon-cpp/pull/222#discussion_r3954193706
##########
src/paimon/rest/rest_catalog.cpp:
##########
@@ -404,8 +410,28 @@ Result<std::shared_ptr<Schema>>
RestCatalog::LoadTableSchema(const Identifier& i
return checked_pointer_cast<Schema>(schema);
}
+Result<std::shared_ptr<FormatTable>> RestCatalog::LoadFormatTable(
+ const Identifier& identifier) const {
+ PAIMON_ASSIGN_OR_RAISE(bool is_system_table, identifier.IsSystemTable());
+ if (is_system_table ||
CatalogUtils::IsSystemDatabase(identifier.GetDatabaseName())) {
+ return Status::Invalid(fmt::format("{} is a system table, so it cannot
be a format table",
+ identifier.GetFullName()));
+ }
Review Comment:
Done. `Catalog::GetFormatTable()` runs `CatalogUtils::CheckNotSystemTable()`
for every catalog before dispatching (`catalog.cpp:71`), so the system database
is checked before the name is parsed and `sys.<malformed>` reports the
system-table refusal rather than a parse error. The hand-rolled pair in
`rest_catalog.cpp` is gone — `RestCatalog::LoadFormatTable()` only loads now.
##########
src/paimon/core/utils/partition_path_utils.cpp:
##########
@@ -49,8 +50,17 @@ const std::bitset<128>& PartitionPathUtils::CharToEscape() {
return bitset;
}
+Status PartitionPathUtils::ValidatePartitionValueForPath(const std::string&
value,
+ bool only_value) {
+ if (value.empty() || (only_value && (value == "." || value == ".."))) {
+ return Status::Invalid("Partition value '" + value +
+ "' cannot be used as a partition path
component.");
Review Comment:
Done — switched to `fmt::format()`.
##########
src/paimon/core/table/format/format_commit_message.h:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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 <cstdint>
+#include <map>
+#include <string>
+
+#include "paimon/commit_message.h"
+
+namespace paimon {
+
+/// One file a `FormatTableWrite` has written but not yet published.
+///
+/// The file is complete on disk under `temp_file_path`, which a scan skips;
committing renames it
+/// to `file_path`, which is what makes it part of the table.
+///
+/// It is a `CommitMessage` so that a format table can be written and
committed through
+/// `FileStoreWrite` and `FileStoreCommit` like any other table, as Java
Paimon's
+/// `TwoPhaseCommitMessage` is. It names a staged path rather than files to
record in a manifest,
+/// so `CommitMessage::Serialize()` refuses it: there is no cross-runtime
encoding for one, and a
+/// write and its commit belong to the same process.
+struct FormatCommitMessage : public CommitMessage {
+ FormatCommitMessage(const std::string& _temp_file_path, const std::string&
_file_path,
+ const std::map<std::string, std::string>& _partition,
int64_t _record_count,
+ int64_t _file_size)
+ : temp_file_path(_temp_file_path),
+ file_path(_file_path),
+ partition(_partition),
+ record_count(_record_count),
+ file_size(_file_size) {}
+
+ ~FormatCommitMessage() override = default;
+
+ std::string ToString() const;
Review Comment:
Done — `ToString()` is defined inline in `format_commit_message.h` now, with
`fmt/format.h` included there, so the header carries its own implementation.
--
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]
