SteNicholas commented on code in PR #222:
URL: https://github.com/apache/paimon-cpp/pull/222#discussion_r3840609210
##########
src/paimon/core/schema/schema_validation.cpp:
##########
@@ -134,27 +143,132 @@ bool SchemaValidation::IsComplexType(const
std::shared_ptr<arrow::Field>& field)
BlobUtils::IsBlobField(field));
}
-Status SchemaValidation::ValidateTableSchema(const TableSchema& schema) {
- const auto& field_names = schema.FieldNames();
- PAIMON_RETURN_NOT_OK(ValidateNoDuplicateField(schema.BucketKeys(), "bucket
key"));
- PAIMON_RETURN_NOT_OK(ValidateNoDuplicateField(schema.PrimaryKeys(),
"primary key"));
- PAIMON_RETURN_NOT_OK(ValidateNoDuplicateField(schema.PartitionKeys(),
"partition key"));
- PAIMON_RETURN_NOT_OK(
- Preconditions::CheckState(ObjectUtils::ContainsAll(field_names,
schema.PartitionKeys()),
- "Table column {} should include all
partition fields {}",
- field_names, schema.PartitionKeys()));
- PAIMON_RETURN_NOT_OK(
- Preconditions::CheckState(ObjectUtils::ContainsAll(field_names,
schema.PrimaryKeys()),
- "Table column {} should include all primary
key constraint {}",
- field_names, schema.PrimaryKeys()));
+Status SchemaValidation::ValidateKeyFieldNames(const std::vector<std::string>&
field_names,
+ const std::vector<std::string>&
bucket_keys,
+ const std::vector<std::string>&
primary_keys,
+ const std::vector<std::string>&
partition_keys) {
+ PAIMON_RETURN_NOT_OK(ValidateNoDuplicateField(bucket_keys, "bucket key"));
+ PAIMON_RETURN_NOT_OK(ValidateNoDuplicateField(primary_keys, "primary
key"));
+ PAIMON_RETURN_NOT_OK(ValidateNoDuplicateField(partition_keys, "partition
key"));
+ PAIMON_RETURN_NOT_OK(Preconditions::CheckState(
+ ObjectUtils::ContainsAll(field_names, partition_keys),
+ "Table column {} should include all partition fields {}", field_names,
partition_keys));
+ PAIMON_RETURN_NOT_OK(Preconditions::CheckState(
+ ObjectUtils::ContainsAll(field_names, primary_keys),
+ "Table column {} should include all primary key constraint {}",
field_names, primary_keys));
+ for (const auto& field_name : field_names) {
+ if (SpecialFields::IsSystemField(field_name)) {
+ return Status::Invalid(
+ fmt::format("field name '{}' in schema cannot be special
field.", field_name));
+ }
+ }
+ return Status::OK();
+}
+Status SchemaValidation::ValidateNewTableSchema(const TableSchema& schema) {
+ const std::map<std::string, std::string>& options = schema.Options();
+ PAIMON_ASSIGN_OR_RAISE(TableType table_type,
TableTypeDefine::FromOptions(options));
+ if (table_type != TableType::TABLE && table_type !=
TableType::MATERIALIZED_TABLE &&
+ table_type != TableType::FORMAT_TABLE) {
+ // Quoted back rather than re-rendered from `table_type`, so the
message says what was
+ // actually asked for.
+ auto type_iter = options.find(Options::TYPE);
+ return Status::NotImplemented(fmt::format(
+ "Cannot create a table whose '{}' is '{}': paimon-cpp does not
implement this table "
+ "type.",
+ Options::TYPE, type_iter == options.end() ? std::string() :
type_iter->second));
+ }
+ if (table_type == TableType::FORMAT_TABLE) {
+ PAIMON_RETURN_NOT_OK(ValidateGenericTableSchema(schema));
+ return ValidateFormatTableSchema(schema);
+ }
+ return ValidateTableSchema(schema);
+}
+
+Status SchemaValidation::ValidateGenericTableSchema(const TableSchema& schema)
{
+ PAIMON_RETURN_NOT_OK(ValidateKeyFieldNames(schema.FieldNames(),
schema.BucketKeys(),
+ schema.PrimaryKeys(),
schema.PartitionKeys()));
PAIMON_RETURN_NOT_OK(
ValidateOnlyContainPrimitiveType(schema.Fields(),
schema.PrimaryKeys(), "primary key"));
PAIMON_RETURN_NOT_OK(
ValidateOnlyContainPrimitiveType(schema.Fields(),
schema.PartitionKeys(), "partition"));
// TODO(lisizhuo.lsz): C++ Paimon do not support timestamp & decimal &
float & double type in
// partition keys for now.
PAIMON_RETURN_NOT_OK(ValidateNotContainSpecificType(schema.Fields(),
schema.PartitionKeys()));
+ return Status::OK();
+}
+
+Status SchemaValidation::ValidateGenericDataSchema(const DataSchema& schema) {
+ PAIMON_RETURN_NOT_OK(ValidateKeyFieldNames(schema.FieldNames(),
schema.BucketKeys(),
+ schema.PrimaryKeys(),
schema.PartitionKeys()));
+ // A `DataSchema` carries its field types as an arrow schema, so the type
rules read them from
+ // there. Only the partition and primary keys are looked up, and both were
just checked.
+ PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<::ArrowSchema> c_schema,
schema.GetArrowSchema());
+ ScopeGuard schema_guard([&c_schema]() {
ArrowSchemaRelease(c_schema.get()); });
+ PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<arrow::Schema>
arrow_schema,
+ arrow::ImportSchema(c_schema.get()));
+ PAIMON_RETURN_NOT_OK(
+ ValidateOnlyContainPrimitiveType(*arrow_schema, schema.PrimaryKeys(),
"primary key"));
+ PAIMON_RETURN_NOT_OK(
+ ValidateOnlyContainPrimitiveType(*arrow_schema,
schema.PartitionKeys(), "partition"));
+ PAIMON_RETURN_NOT_OK(ValidateNotContainSpecificType(*arrow_schema,
schema.PartitionKeys()));
+ return Status::OK();
+}
+
+Status SchemaValidation::ValidateFormatTableSchema(const DataSchema& schema) {
+ // Runs both when the table is created and when it is opened: creation
alone would let a
+ // schema written elsewhere through, and opening alone would persist a
table nothing can load.
+ if (!schema.PrimaryKeys().empty()) {
+ return Status::Invalid(
+ "Cannot define primary keys for a format table: a directory of
data files records no "
+ "row identity to merge on.");
+ }
+
+ const std::map<std::string, std::string>& options = schema.Options();
+ PAIMON_ASSIGN_OR_RAISE(std::string file_format,
OptionsUtils::GetValueFromMap<std::string>(
+ options,
Options::FILE_FORMAT, "parquet"));
+ // The parsed value is not kept; this only has to fail for a format
nothing here can read.
+ PAIMON_RETURN_NOT_OK(FormatTable::ParseFormat(file_format).status());
Review Comment:
Done.
##########
src/paimon/core/schema/schema_validation.cpp:
##########
@@ -134,27 +143,132 @@ bool SchemaValidation::IsComplexType(const
std::shared_ptr<arrow::Field>& field)
BlobUtils::IsBlobField(field));
}
-Status SchemaValidation::ValidateTableSchema(const TableSchema& schema) {
- const auto& field_names = schema.FieldNames();
- PAIMON_RETURN_NOT_OK(ValidateNoDuplicateField(schema.BucketKeys(), "bucket
key"));
- PAIMON_RETURN_NOT_OK(ValidateNoDuplicateField(schema.PrimaryKeys(),
"primary key"));
- PAIMON_RETURN_NOT_OK(ValidateNoDuplicateField(schema.PartitionKeys(),
"partition key"));
- PAIMON_RETURN_NOT_OK(
- Preconditions::CheckState(ObjectUtils::ContainsAll(field_names,
schema.PartitionKeys()),
- "Table column {} should include all
partition fields {}",
- field_names, schema.PartitionKeys()));
- PAIMON_RETURN_NOT_OK(
- Preconditions::CheckState(ObjectUtils::ContainsAll(field_names,
schema.PrimaryKeys()),
- "Table column {} should include all primary
key constraint {}",
- field_names, schema.PrimaryKeys()));
+Status SchemaValidation::ValidateKeyFieldNames(const std::vector<std::string>&
field_names,
+ const std::vector<std::string>&
bucket_keys,
+ const std::vector<std::string>&
primary_keys,
+ const std::vector<std::string>&
partition_keys) {
+ PAIMON_RETURN_NOT_OK(ValidateNoDuplicateField(bucket_keys, "bucket key"));
+ PAIMON_RETURN_NOT_OK(ValidateNoDuplicateField(primary_keys, "primary
key"));
+ PAIMON_RETURN_NOT_OK(ValidateNoDuplicateField(partition_keys, "partition
key"));
+ PAIMON_RETURN_NOT_OK(Preconditions::CheckState(
+ ObjectUtils::ContainsAll(field_names, partition_keys),
+ "Table column {} should include all partition fields {}", field_names,
partition_keys));
+ PAIMON_RETURN_NOT_OK(Preconditions::CheckState(
+ ObjectUtils::ContainsAll(field_names, primary_keys),
+ "Table column {} should include all primary key constraint {}",
field_names, primary_keys));
+ for (const auto& field_name : field_names) {
+ if (SpecialFields::IsSystemField(field_name)) {
+ return Status::Invalid(
+ fmt::format("field name '{}' in schema cannot be special
field.", field_name));
+ }
+ }
+ return Status::OK();
+}
+Status SchemaValidation::ValidateNewTableSchema(const TableSchema& schema) {
+ const std::map<std::string, std::string>& options = schema.Options();
+ PAIMON_ASSIGN_OR_RAISE(TableType table_type,
TableTypeDefine::FromOptions(options));
+ if (table_type != TableType::TABLE && table_type !=
TableType::MATERIALIZED_TABLE &&
+ table_type != TableType::FORMAT_TABLE) {
+ // Quoted back rather than re-rendered from `table_type`, so the
message says what was
+ // actually asked for.
+ auto type_iter = options.find(Options::TYPE);
+ return Status::NotImplemented(fmt::format(
+ "Cannot create a table whose '{}' is '{}': paimon-cpp does not
implement this table "
+ "type.",
+ Options::TYPE, type_iter == options.end() ? std::string() :
type_iter->second));
+ }
+ if (table_type == TableType::FORMAT_TABLE) {
+ PAIMON_RETURN_NOT_OK(ValidateGenericTableSchema(schema));
+ return ValidateFormatTableSchema(schema);
+ }
+ return ValidateTableSchema(schema);
+}
+
+Status SchemaValidation::ValidateGenericTableSchema(const TableSchema& schema)
{
+ PAIMON_RETURN_NOT_OK(ValidateKeyFieldNames(schema.FieldNames(),
schema.BucketKeys(),
+ schema.PrimaryKeys(),
schema.PartitionKeys()));
PAIMON_RETURN_NOT_OK(
ValidateOnlyContainPrimitiveType(schema.Fields(),
schema.PrimaryKeys(), "primary key"));
PAIMON_RETURN_NOT_OK(
ValidateOnlyContainPrimitiveType(schema.Fields(),
schema.PartitionKeys(), "partition"));
// TODO(lisizhuo.lsz): C++ Paimon do not support timestamp & decimal &
float & double type in
// partition keys for now.
PAIMON_RETURN_NOT_OK(ValidateNotContainSpecificType(schema.Fields(),
schema.PartitionKeys()));
+ return Status::OK();
+}
+
+Status SchemaValidation::ValidateGenericDataSchema(const DataSchema& schema) {
+ PAIMON_RETURN_NOT_OK(ValidateKeyFieldNames(schema.FieldNames(),
schema.BucketKeys(),
+ schema.PrimaryKeys(),
schema.PartitionKeys()));
+ // A `DataSchema` carries its field types as an arrow schema, so the type
rules read them from
+ // there. Only the partition and primary keys are looked up, and both were
just checked.
+ PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<::ArrowSchema> c_schema,
schema.GetArrowSchema());
+ ScopeGuard schema_guard([&c_schema]() {
ArrowSchemaRelease(c_schema.get()); });
+ PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<arrow::Schema>
arrow_schema,
+ arrow::ImportSchema(c_schema.get()));
+ PAIMON_RETURN_NOT_OK(
+ ValidateOnlyContainPrimitiveType(*arrow_schema, schema.PrimaryKeys(),
"primary key"));
+ PAIMON_RETURN_NOT_OK(
+ ValidateOnlyContainPrimitiveType(*arrow_schema,
schema.PartitionKeys(), "partition"));
+ PAIMON_RETURN_NOT_OK(ValidateNotContainSpecificType(*arrow_schema,
schema.PartitionKeys()));
+ return Status::OK();
Review Comment:
Done. The `arrow::Schema` overloads are gone. `ValidateGenericDataSchema()`
converts through `DataField::ConvertArrowSchemaToDataFields()`, and it and
`ValidateGenericTableSchema()` now share one `ValidateGenericSchema(fields,
bucket_keys, primary_keys, partition_keys)`.
##########
src/paimon/core/table/format/format_file_listing.h:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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 <memory>
+#include <string>
+#include <vector>
+
+#include "paimon/status.h"
+#include "paimon/table/format/format_data_split.h"
+
+namespace paimon {
+
+class FileSystem;
+
+/// Whether `name` is a directory this library keeps under a format table's
location as metadata.
+/// A table whose schema lives in a catalog has none.
+bool IsReservedFormatTableDirectory(const std::string& name);
+
Review Comment:
Done. `FormatFileListing` is a utility class with static
`IsReservedDirectory()` and `ListDataFiles()`.
##########
src/paimon/core/table/format/format_file_listing_test.cpp:
##########
@@ -0,0 +1,162 @@
+/*
+ * 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 "paimon/core/table/format/format_file_listing.h"
+
+#include <algorithm>
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "gtest/gtest.h"
+#include "paimon/common/utils/path_util.h"
+#include "paimon/fs/file_system.h"
+#include "paimon/fs/local/local_file_system.h"
+#include "paimon/testing/utils/testharness.h"
+
+namespace paimon::test {
+
+namespace {
+
+Status WriteAt(const std::shared_ptr<FileSystem>& file_system, const
std::string& path) {
+ std::string parent = PathUtil::GetParentDirPath(path);
+ PAIMON_RETURN_NOT_OK(file_system->Mkdirs(parent));
+ return file_system->WriteFile(path, "row\n", /*overwrite=*/true);
+}
+
+/// The listed files as paths relative to `root`, sorted so the order of a
listing cannot matter.
+Result<std::vector<std::string>> ListNames(const std::shared_ptr<FileSystem>&
file_system,
+ const std::string& root,
+ const FormatDataFileListingOptions&
options) {
+ std::vector<FormatDataSplit::FileMeta> files;
+ PAIMON_RETURN_NOT_OK(ListFormatDataFiles(file_system, root, options,
&files));
+ std::vector<std::string> names;
+ names.reserve(files.size());
+ for (const FormatDataSplit::FileMeta& file : files) {
+ names.push_back(file.file_path.substr(root.size() + 1));
+ }
+ std::sort(names.begin(), names.end());
+ return names;
+}
+
+} // namespace
+
+TEST(FormatFileListingTest, TestDescendsIntoPlainSubdirectories) {
+ std::unique_ptr<UniqueTestDirectory> dir = UniqueTestDirectory::Create();
+ ASSERT_TRUE(dir);
+ std::shared_ptr<FileSystem> file_system =
std::make_shared<LocalFileSystem>();
+ // `data-file.path-directory`, and other engines, put data files below the
partition directory
+ // rather than directly in it, so stopping at the top level would miss
them.
+ ASSERT_OK(WriteAt(file_system, dir->Str() + "/a.parquet"));
+ ASSERT_OK(WriteAt(file_system, dir->Str() + "/nested/b.parquet"));
+ ASSERT_OK(WriteAt(file_system, dir->Str() + "/nested/deeper/c.parquet"));
+
+ ASSERT_OK_AND_ASSIGN(std::vector<std::string> names,
+ ListNames(file_system, dir->Str(),
FormatDataFileListingOptions{}));
+ ASSERT_EQ(names, (std::vector<std::string>{"a.parquet", "nested/b.parquet",
+ "nested/deeper/c.parquet"}));
+}
+
+TEST(FormatFileListingTest, TestHiddenNamesAreSkippedAndNotDescendedInto) {
+ std::unique_ptr<UniqueTestDirectory> dir = UniqueTestDirectory::Create();
+ ASSERT_TRUE(dir);
+ std::shared_ptr<FileSystem> file_system =
std::make_shared<LocalFileSystem>();
+ // A staging tree holds another job's uncommitted output under ordinary
data file names, so
+ // only the directory above them tells the two apart.
+ ASSERT_OK(WriteAt(file_system, dir->Str() + "/a.parquet"));
+ ASSERT_OK(WriteAt(file_system, dir->Str() + "/.b.parquet.tmp"));
+ ASSERT_OK(WriteAt(file_system, dir->Str() + "/_temporary/c.parquet"));
+ ASSERT_OK(WriteAt(file_system, dir->Str() + "/.hive-staging_1/d.parquet"));
+
+ ASSERT_OK_AND_ASSIGN(std::vector<std::string> names,
+ ListNames(file_system, dir->Str(),
FormatDataFileListingOptions{}));
+ ASSERT_EQ(names, (std::vector<std::string>{"a.parquet"}));
+}
+
+TEST(FormatFileListingTest,
TestDefaultPartitionDirectoryIsTheOneHiddenNameThatIsContent) {
+ std::unique_ptr<UniqueTestDirectory> dir = UniqueTestDirectory::Create();
+ ASSERT_TRUE(dir);
+ std::shared_ptr<FileSystem> file_system =
std::make_shared<LocalFileSystem>();
+ // In the value-only layout a partition directory is the bare value, so a
null partition is
+ // named `__DEFAULT_PARTITION__` - a hidden name that nonetheless holds
table data.
+ ASSERT_OK(WriteAt(file_system, dir->Str() +
"/__DEFAULT_PARTITION__/a.parquet"));
+ ASSERT_OK(WriteAt(file_system, dir->Str() + "/_temporary/b.parquet"));
+
+ FormatDataFileListingOptions options;
+ options.partition_levels = 1;
+ options.only_value_in_path = true;
+ options.default_part_name = "__DEFAULT_PARTITION__";
+ ASSERT_OK_AND_ASSIGN(std::vector<std::string> names,
+ ListNames(file_system, dir->Str(), options));
+ ASSERT_EQ(names,
(std::vector<std::string>{"__DEFAULT_PARTITION__/a.parquet"}));
+
+ // With no partition level below the root, that name is a staging tree
like any other.
+ FormatDataFileListingOptions no_partition_level = options;
+ no_partition_level.partition_levels = 0;
+ ASSERT_OK_AND_ASSIGN(std::vector<std::string> without,
+ ListNames(file_system, dir->Str(),
no_partition_level));
+ ASSERT_TRUE(without.empty());
+}
+
+TEST(FormatFileListingTest,
TestReservedDirectoriesAreSkippedOnlyWhenTheyAreMetadata) {
+ std::unique_ptr<UniqueTestDirectory> dir = UniqueTestDirectory::Create();
+ ASSERT_TRUE(dir);
+ std::shared_ptr<FileSystem> file_system =
std::make_shared<LocalFileSystem>();
+ ASSERT_OK(WriteAt(file_system, dir->Str() + "/a.parquet"));
+ ASSERT_OK(WriteAt(file_system, dir->Str() + "/schema/schema-0"));
+
+ FormatDataFileListingOptions metadata_here;
+ metadata_here.skip_reserved_directories = true;
+ ASSERT_OK_AND_ASSIGN(std::vector<std::string> skipped,
+ ListNames(file_system, dir->Str(), metadata_here));
+ ASSERT_EQ(skipped, (std::vector<std::string>{"a.parquet"}));
+
+ // For a table whose schema lives in a metastore, the location is nothing
but data and a
+ // directory of that name is a partition value or a data subdirectory.
+ ASSERT_OK_AND_ASSIGN(std::vector<std::string> kept,
+ ListNames(file_system, dir->Str(),
FormatDataFileListingOptions{}));
+ ASSERT_EQ(kept, (std::vector<std::string>{"a.parquet",
"schema/schema-0"}));
+}
+
+TEST(FormatFileListingTest,
TestMissingRootIsAnErrorButAVanishedSubdirectoryIsNot) {
+ std::unique_ptr<UniqueTestDirectory> dir = UniqueTestDirectory::Create();
+ ASSERT_TRUE(dir);
+ std::shared_ptr<FileSystem> file_system =
std::make_shared<LocalFileSystem>();
+ // A root that is not there means the location is wrong or the data is
gone. The file systems
+ // here report a missing directory as an empty listing, so this only works
because the root is
+ // asked about outright - passing it off as a table with no rows would
hide a mistyped path.
+ std::vector<FormatDataSplit::FileMeta> files;
+ ASSERT_FALSE(ListFormatDataFiles(file_system, dir->Str() + "/absent",
+ FormatDataFileListingOptions{}, &files)
+ .ok());
+
Review Comment:
Done.
##########
src/paimon/core/table/format/format_file_naming.h:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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 <string>
+
+#include "paimon/result.h"
+
+namespace paimon {
+
+/// Names the data files one format table write produces.
+///
+/// `{prefix}{uuid}-{n}.{extension}`, the convention every paimon writer
follows: the uuid belongs
+/// to this write and the counter to its files, so two concurrent writers
cannot collide.
+///
+/// A file is written under a hidden `.{name}.tmp` and takes its real name
only on commit, both
+/// marks a scan and an orphan-file cleaner already treat as uncommitted
output.
+class FormatFileNaming {
+ public:
+ static constexpr char DEFAULT_DATA_FILE_PREFIX[] = "data-";
+ static constexpr char TEMP_FILE_SUFFIX[] = ".tmp";
+
+ /// @param extension File extension without its dot, which is the format's
identifier.
+ /// @param prefix File name prefix, from `data-file.prefix`. It may not be
hidden by the
+ /// `_` / `.` convention, since a scan skips every such file.
+ static Result<FormatFileNaming> Create(const std::string& extension,
+ const std::string& prefix =
DEFAULT_DATA_FILE_PREFIX);
+
Review Comment:
Done. `FormatFileNaming::Create()` takes the data file prefix rather than
defaulting it, and `SchemaValidation::ValidateFormatTableSchema()` takes the
file system explicitly as well.
##########
src/paimon/core/table/format/format_file_naming.h:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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 <string>
+
+#include "paimon/result.h"
+
+namespace paimon {
+
+/// Names the data files one format table write produces.
+///
+/// `{prefix}{uuid}-{n}.{extension}`, the convention every paimon writer
follows: the uuid belongs
+/// to this write and the counter to its files, so two concurrent writers
cannot collide.
+///
+/// A file is written under a hidden `.{name}.tmp` and takes its real name
only on commit, both
+/// marks a scan and an orphan-file cleaner already treat as uncommitted
output.
+class FormatFileNaming {
+ public:
+ static constexpr char DEFAULT_DATA_FILE_PREFIX[] = "data-";
+ static constexpr char TEMP_FILE_SUFFIX[] = ".tmp";
+
Review Comment:
Done. `kDefaultDataFilePrefix`, `kTempDirName` and `kTempFilePrefix` here,
and `TableTypeDefine`'s identifiers were renamed the same way (`kTable`,
`kFormatTable`, `kMaterializedTable`, `kObjectTable`, `kLanceTable`,
`kIcebergTable`).
##########
src/paimon/core/table/format/format_file_naming.h:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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 <string>
+
+#include "paimon/result.h"
+
+namespace paimon {
+
+/// Names the data files one format table write produces.
+///
+/// `{prefix}{uuid}-{n}.{extension}`, the convention every paimon writer
follows: the uuid belongs
+/// to this write and the counter to its files, so two concurrent writers
cannot collide.
+///
+/// A file is written under a hidden `.{name}.tmp` and takes its real name
only on commit, both
+/// marks a scan and an orphan-file cleaner already treat as uncommitted
output.
+class FormatFileNaming {
+ public:
+ static constexpr char DEFAULT_DATA_FILE_PREFIX[] = "data-";
+ static constexpr char TEMP_FILE_SUFFIX[] = ".tmp";
+
+ /// @param extension File extension without its dot, which is the format's
identifier.
+ /// @param prefix File name prefix, from `data-file.prefix`. It may not be
hidden by the
+ /// `_` / `.` convention, since a scan skips every such file.
+ static Result<FormatFileNaming> Create(const std::string& extension,
+ const std::string& prefix =
DEFAULT_DATA_FILE_PREFIX);
+
+ FormatFileNaming() = default;
+
+ /// The name the next file takes once committed.
+ std::string NextFileName();
+
+ /// The hidden name a file is written under before it is committed.
+ static std::string ToTempFileName(const std::string& file_name) {
+ return "." + file_name + TEMP_FILE_SUFFIX;
+ }
Review Comment:
Aligned with Java. A file is staged under `<publish
directory>/_temporary/.tmp.<uuid>`, which is what
`RenamingTwoPhaseOutputStream.generateTempPath()` produces, and the commit
renames it into place. `FormatFileNaming::IsTempFilePath()` accepts only that
shape, so a plain hidden name beside the target, a name outside `_temporary`
and a deeper staging tree are all refused.
`TestTempPathIsAHiddenNameInATemporaryDirectory` covers it.
--
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]
