Schema::ValidateIdentifierFields as a public static func

Author: WZhuo

src/iceberg/schema.cc

@@ -35,10 +35,53 @@
 
 namespace iceberg {
 
+Schema::Schema(std::vector<SchemaField> fields, int32_t schema_id)
+    : StructType(std::move(fields)), schema_id_(schema_id) {}
+
+Result<std::unique_ptr<Schema>> Schema::Make(std::vector<SchemaField> fields,
+                                             int32_t schema_id,
+                                             std::vector<int32_t> identifier_field_ids) {
+  auto schema = std::make_unique<Schema>(std::move(fields), schema_id);
 
+  if (!identifier_field_ids.empty()) {
+    auto id_to_parent = IndexParents(*schema);
+    for (auto field_id : identifier_field_ids) {
+      ICEBERG_RETURN_UNEXPECTED(
+          ValidateIdentifierFields(field_id, *schema, id_to_parent));
+    }
+  }
+
+  schema->identifier_field_ids_ = std::move(identifier_field_ids);
+  return schema;
+}
+
+Result<std::unique_ptr<Schema>> Schema::Make(
+    std::vector<SchemaField> fields, int32_t schema_id,
+    const std::vector<std::string>& identifier_field_names) {
+  auto schema = std::make_unique<Schema>(std::move(fields), schema_id);
 
-namespace {
-Status ValidateIdentifierFields(
+  std::vector<int32_t> fresh_identifier_ids;
+  for (const auto& name : identifier_field_names) {
+    ICEBERG_ASSIGN_OR_RAISE(auto field, schema->FindFieldByName(name));
+    if (!field) {
+      return InvalidSchema("Cannot find identifier field: {}", name);
+    }
+    fresh_identifier_ids.push_back(field.value().get().field_id());
+  }
+
+  if (!fresh_identifier_ids.empty()) {
+    auto id_to_parent = IndexParents(*schema);
+    for (auto field_id : fresh_identifier_ids) {
+      ICEBERG_RETURN_UNEXPECTED(
+          ValidateIdentifierFields(field_id, *schema, id_to_parent));
+    }
+  }
+
+  schema->identifier_field_ids_ = std::move(fresh_identifier_ids);
+  return schema;
+}
+
+Status Schema::ValidateIdentifierFields(
     int32_t field_id, const Schema& schema,
     const std::unordered_map<int32_t, int32_t>& id_to_parent) {
   ICEBERG_ASSIGN_OR_RAISE(auto field_opt, schema.FindFieldById(field_id));
@@ -87,53 +130,6 @@ Status ValidateIdentifierFields(
   }
   return {};
 }
-}  // namespace
-
-Schema::Schema(std::vector<SchemaField> fields, int32_t schema_id)
-    : StructType(std::move(fields)), schema_id_(schema_id) {}
-
-Result<std::unique_ptr<Schema>> Schema::Make(std::vector<SchemaField> fields,
-                                             int32_t schema_id,
-                                             std::vector<int32_t> identifier_field_ids) {
-  auto schema = std::make_unique<Schema>(std::move(fields), schema_id);
-
-  if (!identifier_field_ids.empty()) {
-    auto id_to_parent = IndexParents(*schema);
-    for (auto field_id : identifier_field_ids) {
-      ICEBERG_RETURN_UNEXPECTED(
-          ValidateIdentifierFields(field_id, *schema, id_to_parent));
-    }
-  }
-
-  schema->identifier_field_ids_ = std::move(identifier_field_ids);
-  return schema;
-}
-
-Result<std::unique_ptr<Schema>> Schema::Make(
-    std::vector<SchemaField> fields, int32_t schema_id,
-    const std::vector<std::string>& identifier_field_names) {
-  auto schema = std::make_unique<Schema>(std::move(fields), schema_id);
-
-  std::vector<int32_t> fresh_identifier_ids;
-  for (const auto& name : identifier_field_names) {
-    ICEBERG_ASSIGN_OR_RAISE(auto field, schema->FindFieldByName(name));
-    if (!field) {
-      return InvalidSchema("Cannot find identifier field: {}", name);
-    }
-    fresh_identifier_ids.push_back(field.value().get().field_id());
-  }
-
-  if (!fresh_identifier_ids.empty()) {
-    auto id_to_parent = IndexParents(*schema);
-    for (auto field_id : fresh_identifier_ids) {
-      ICEBERG_RETURN_UNEXPECTED(
-          ValidateIdentifierFields(field_id, *schema, id_to_parent));
-    }
-  }
-
-  schema->identifier_field_ids_ = std::move(fresh_identifier_ids);
-  return schema;
-}
 
 const std::shared_ptr<Schema>& Schema::EmptySchema() {
   static const auto empty_schema =

src/iceberg/schema.h

@@ -58,8 +58,7 @@ class ICEBERG_EXPORT Schema : public StructType {
   ///
   /// \param fields The fields that make up the schema.
   /// \param schema_id The unique identifier for this schema (default:kInitialSchemaId).
-  /// \param identifier_field_ids Field IDs that uniquely identify
-  /// rows in the table (default: empty).
+  /// \param identifier_field_ids Field IDs that uniquely identify rows in the table.
   /// \return A new Schema instance or Status if failed.
   static Result<std::unique_ptr<Schema>> Make(std::vector<SchemaField> fields,
                                               int32_t schema_id,
@@ -70,12 +69,31 @@ class ICEBERG_EXPORT Schema : public StructType {
   /// \param fields The fields that make up the schema.
   /// \param schema_id The unique identifier for this schema (default: kInitialSchemaId).
   /// \param identifier_field_names Canonical names of fields that uniquely identify rows
-  /// in the table (default: empty).
+  /// in the table.
   /// \return A new Schema instance or Status if failed.
   static Result<std::unique_ptr<Schema>> Make(
       std::vector<SchemaField> fields, int32_t schema_id,
       const std::vector<std::string>& identifier_field_names);
 
+  /// \brief Validate that the identifier field with the given ID is valid for the schema
+  ///
+  /// This method checks that the specified field ID represents a valid identifier field
+  /// according to Iceberg's identifier field requirements. It verifies that the field:
+  /// - exists in the schema
+  /// - is a primitive type
+  /// - is not optional (required field)
+  /// - is not a float or double type
+  /// - is not nested within optional or non-struct parent fields
+  ///
+  /// \param field_id The ID of the field to validate as an identifier field.
+  /// \param schema The schema containing the field to validate.
+  /// \param id_to_parent A mapping from field IDs to their parent field IDs for nested
+  /// field validation.
+  /// \return Status indicating success or failure of the validation.
+  static Status ValidateIdentifierFields(
+      int32_t field_id, const Schema& schema,
+      const std::unordered_map<int32_t, int32_t>& id_to_parent);
+
   /// \brief Get an empty schema.
   ///
   /// An empty schema has no fields and a schema ID of 0.