wgtmac commented on code in PR #430:
URL: https://github.com/apache/iceberg-cpp/pull/430#discussion_r2643196804
##########
src/iceberg/schema.h:
##########
@@ -103,26 +121,45 @@ class ICEBERG_EXPORT Schema : public StructType {
Result<std::unique_ptr<Schema>> Project(
const std::unordered_set<int32_t>& field_ids) const;
+ const std::vector<int32_t>& IdentifierFieldIds() const;
+ Result<std::vector<std::string>> IdentifierFieldNames() const;
Review Comment:
```suggestion
/// \brief Returns field ids of identifier fields.
const std::vector<int32_t>& IdentifierFieldIds() const;
/// \brief Returns canonical field names of identifier fields.
Result<std::vector<std::string>> IdentifierFieldNames() const;
```
##########
src/iceberg/schema.h:
##########
@@ -78,6 +90,12 @@ class ICEBERG_EXPORT Schema : public StructType {
Result<std::optional<std::reference_wrapper<const SchemaField>>>
FindFieldById(
int32_t field_id) const;
+ /// \brief Returns the full column name for the given id.
Review Comment:
```suggestion
/// \brief Returns the canonical column name for the given id.
```
##########
src/iceberg/schema.h:
##########
@@ -49,7 +49,19 @@ class ICEBERG_EXPORT Schema : public StructType {
static constexpr int32_t kInvalidColumnId = -1;
explicit Schema(std::vector<SchemaField> fields,
- std::optional<int32_t> schema_id = std::nullopt);
+ std::optional<int32_t> schema_id = std::nullopt,
+ std::vector<int32_t> identifier_field_ids = {});
+
+ /// \brief Create a schema.
+ ///
+ /// \param fields The fields that make up the schema.
+ /// \param schema_id The unique identifier for this schema (default:
kInitialSchemaId).
Review Comment:
I think it is better to keep `std::nullopt` as the default or remove the
default value of `schema_id`. Otherwise users may accidentally use
`kInitialSchemaId` as the schema_id.
--
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]
