Copilot commented on code in PR #801:
URL: https://github.com/apache/iceberg-cpp/pull/801#discussion_r3689114514
##########
src/iceberg/inspect/metadata_table.h:
##########
@@ -20,46 +20,109 @@
#pragma once
/// \file iceberg/inspect/metadata_table.h
-/// \brief Define base APIs for metadata tables.
+/// \brief Base APIs for inspecting Iceberg metadata tables.
+#include <concepts>
#include <memory>
+#include <string>
+#include <utility>
+#include <variant>
+#include "iceberg/arrow_c_data.h"
#include "iceberg/iceberg_export.h"
#include "iceberg/result.h"
-#include "iceberg/table_identifier.h"
#include "iceberg/type_fwd.h"
+#include "iceberg/util/timepoint.h"
namespace iceberg {
-/// \brief Base class for Iceberg metadata tables.
+/// \brief Base interface for an Iceberg metadata table.
class ICEBERG_EXPORT MetadataTable {
public:
+ /// \brief Supported metadata table kinds.
enum class Kind {
kSnapshots,
kHistory,
};
- static Result<std::unique_ptr<MetadataTable>> Make(std::shared_ptr<Table>
table,
- Kind kind);
+ /// \brief Maximum number of rows emitted in each Arrow batch.
+ static constexpr int64_t kBatchSize = 1024;
+ /// \brief Create a metadata table of the requested concrete type.
+ ///
+ /// \tparam MetadataTableType Concrete class derived from MetadataTable.
+ /// \param table Source table whose metadata will be exposed.
+ /// \return The constructed metadata table, or an error.
+ template <typename MetadataTableType>
+ requires std::derived_from<MetadataTableType, MetadataTable>
+ static Result<std::unique_ptr<MetadataTableType>>
Make(std::shared_ptr<Table> table) {
+ return MetadataTableType::Make(std::move(table));
+ }
Review Comment:
This change makes `MetadataTable` construction template-only and (based on
the diff) removes the previous `Make(std::shared_ptr<Table>, Kind)` factory and
`name()` identifier access from the public base API. If `MetadataTable` is a
public API (it is `ICEBERG_EXPORT`), this is a breaking change for downstream
callers. Consider restoring a non-template factory overload (e.g., `Make(table,
Kind)`) and/or keeping a stable identifier accessor (or a clear replacement) to
preserve backward compatibility, or introduce a deprecation path.
--
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]
