HuaHuaY commented on code in PR #418:
URL: https://github.com/apache/iceberg-cpp/pull/418#discussion_r2629778602
##########
src/iceberg/table.cc:
##########
@@ -110,18 +130,78 @@ const std::vector<SnapshotLogEntry>& Table::history()
const {
return metadata_->snapshot_log;
}
-std::unique_ptr<UpdateProperties> Table::UpdateProperties() const {
- return std::make_unique<iceberg::UpdateProperties>(identifier_, catalog_,
metadata_);
+const std::shared_ptr<FileIO>& Table::io() const { return io_; }
+
+const std::shared_ptr<TableMetadata>& Table::metadata() const { return
metadata_; }
+
+const std::shared_ptr<Catalog>& Table::catalog() const { return catalog_; }
+
+Result<std::unique_ptr<TableScanBuilder>> Table::NewScan() const {
+ return std::make_unique<TableScanBuilder>(metadata_, io_);
}
-std::unique_ptr<Transaction> Table::NewTransaction() const {
- throw NotImplemented("Table::NewTransaction is not implemented");
+Result<std::shared_ptr<Transaction>> Table::NewTransaction() {
+ // Create a brand new transaction object for the table. Users are expected
to commit the
+ // transaction manually.
+ return Transaction::Make(shared_from_this(), Transaction::Kind::kUpdate,
+ /*auto_commit=*/false);
}
-const std::shared_ptr<FileIO>& Table::io() const { return io_; }
+Result<std::shared_ptr<UpdateProperties>> Table::NewUpdateProperties() {
+ ICEBERG_ASSIGN_OR_RAISE(
+ auto transaction, Transaction::Make(shared_from_this(),
Transaction::Kind::kUpdate,
+ /*auto_commit=*/true));
+ return transaction->NewUpdateProperties();
+}
-std::unique_ptr<TableScanBuilder> Table::NewScan() const {
- return std::make_unique<TableScanBuilder>(metadata_, io_);
+Result<std::shared_ptr<StagedTable>> StagedTable::Make(
+ TableIdentifier identifier, std::shared_ptr<TableMetadata> metadata,
+ std::string metadata_location, std::shared_ptr<FileIO> io,
+ std::shared_ptr<Catalog> catalog) {
+ if (metadata == nullptr) [[unlikely]] {
+ return InvalidArgument("Metadata cannot be null");
+ }
+ if (io == nullptr) [[unlikely]] {
+ return InvalidArgument("FileIO cannot be null");
+ }
+ if (catalog == nullptr) [[unlikely]] {
+ return InvalidArgument("Catalog cannot be null");
+ }
+ return std::shared_ptr<StagedTable>(
+ new StagedTable(std::move(identifier), std::move(metadata),
+ std::move(metadata_location), std::move(io),
std::move(catalog)));
Review Comment:
```suggestion
return std::make_shared<StagedTable>(std::move(identifier),
std::move(metadata),
std::move(metadata_location),
std::move(io),
std::move(catalog));
```
##########
src/iceberg/update/update_properties.cc:
##########
@@ -19,28 +19,33 @@
#include "iceberg/update/update_properties.h"
+#include <charconv>
#include <cstdint>
#include <memory>
+#include <system_error>
-#include "iceberg/catalog.h"
#include "iceberg/metrics_config.h"
#include "iceberg/result.h"
-#include "iceberg/table.h"
-#include "iceberg/table_identifier.h"
#include "iceberg/table_metadata.h"
#include "iceberg/table_properties.h"
-#include "iceberg/table_requirements.h"
#include "iceberg/table_update.h"
+#include "iceberg/transaction.h"
#include "iceberg/util/macros.h"
namespace iceberg {
-UpdateProperties::UpdateProperties(TableIdentifier identifier,
- std::shared_ptr<Catalog> catalog,
- std::shared_ptr<TableMetadata> base)
- : identifier_(std::move(identifier)),
- catalog_(std::move(catalog)),
- base_metadata_(std::move(base)) {}
+Result<std::shared_ptr<UpdateProperties>> UpdateProperties::Make(
+ std::shared_ptr<Transaction> transaction) {
+ if (!transaction) [[unlikely]] {
+ return InvalidArgument("Cannot create UpdateProperties without a
transaction");
+ }
+ return std::shared_ptr<UpdateProperties>(new
UpdateProperties(std::move(transaction)));
Review Comment:
I suggest to add some comments if we prefer to use `std::shared_ptr` rather
than `std::make_shared` here.
--
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]
