Skip to content

Commit b2e75a8

Browse files
author
shuxu.li
committed
feat: transactional UpdateProperties method support
1 parent 7475e89 commit b2e75a8

File tree

7 files changed

+24
-42
lines changed

7 files changed

+24
-42
lines changed

src/iceberg/base_transaction.cc

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919

2020
#include "iceberg/base_transaction.h"
2121

22-
#include <utility>
23-
2422
#include "iceberg/catalog.h"
2523
#include "iceberg/pending_update.h"
2624
#include "iceberg/table.h"
@@ -30,14 +28,14 @@
3028

3129
namespace iceberg {
3230

33-
BaseTransaction::BaseTransaction(std::shared_ptr<Table> table,
31+
BaseTransaction::BaseTransaction(std::shared_ptr<const Table> table,
3432
std::shared_ptr<Catalog> catalog)
3533
: table_(std::move(table)), catalog_(std::move(catalog)) {
3634
ICEBERG_DCHECK(table_ != nullptr, "table must not be null");
3735
ICEBERG_DCHECK(catalog_ != nullptr, "catalog must not be null");
3836
}
3937

40-
const std::shared_ptr<Table>& BaseTransaction::table() const { return table_; }
38+
const std::shared_ptr<const Table>& BaseTransaction::table() const { return table_; }
4139

4240
std::shared_ptr<PropertiesUpdate> BaseTransaction::UpdateProperties() {
4341
return RegisterUpdate<PropertiesUpdate>();

src/iceberg/base_transaction.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ namespace iceberg {
2929
/// \brief Base class for transaction implementations
3030
class BaseTransaction : public Transaction {
3131
public:
32-
BaseTransaction(std::shared_ptr<Table> table, std::shared_ptr<Catalog> catalog);
32+
BaseTransaction(std::shared_ptr<const Table> table, std::shared_ptr<Catalog> catalog);
3333
~BaseTransaction() override = default;
3434

35-
const std::shared_ptr<Table>& table() const override;
35+
const std::shared_ptr<const Table>& table() const override;
3636

3737
std::shared_ptr<PropertiesUpdate> UpdateProperties() override;
3838

@@ -48,7 +48,7 @@ class BaseTransaction : public Transaction {
4848
return update;
4949
}
5050

51-
std::shared_ptr<Table> table_;
51+
std::shared_ptr<const Table> table_;
5252
std::shared_ptr<Catalog> catalog_;
5353
std::vector<std::shared_ptr<PendingUpdate>> pending_updates_;
5454
};

src/iceberg/pending_update.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,6 @@
2727

2828
namespace iceberg {
2929

30-
// ============================================================================
31-
// UpdateProperties implementation
32-
// ============================================================================
33-
3430
PropertiesUpdate& PropertiesUpdate::Set(std::string const& key,
3531
std::string const& value) {
3632
updates_[key] = value;

src/iceberg/table.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919

2020
#include "iceberg/table.h"
2121

22+
#include <algorithm>
23+
24+
#include "iceberg/base_transaction.h"
2225
#include "iceberg/catalog.h"
2326
#include "iceberg/partition_spec.h"
2427
#include "iceberg/schema.h"
@@ -114,7 +117,7 @@ std::unique_ptr<UpdateProperties> Table::UpdateProperties() const {
114117
}
115118

116119
std::unique_ptr<Transaction> Table::NewTransaction() const {
117-
throw NotImplemented("Table::NewTransaction is not implemented");
120+
return std::make_unique<BaseTransaction>(shared_from_this(), catalog_);
118121
}
119122

120123
const std::shared_ptr<FileIO>& Table::io() const { return io_; }

src/iceberg/table.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
namespace iceberg {
3434

3535
/// \brief Represents an Iceberg table
36-
class ICEBERG_EXPORT Table {
36+
class ICEBERG_EXPORT Table : public std::enable_shared_from_this<Table> {
3737
public:
3838
~Table();
3939

src/iceberg/test/transaction_pending_update_test.cc

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,34 +17,20 @@
1717
* under the License.
1818
*/
1919

20-
#include <chrono>
21-
#include <memory>
22-
#include <string>
23-
#include <utility>
24-
#include <vector>
25-
26-
#include <gmock/gmock.h>
2720
#include <gtest/gtest.h>
2821

2922
#include "iceberg/base_transaction.h"
3023
#include "iceberg/partition_spec.h"
3124
#include "iceberg/pending_update.h"
32-
#include "iceberg/snapshot.h"
3325
#include "iceberg/sort_order.h"
3426
#include "iceberg/table.h"
35-
#include "iceberg/table_identifier.h"
3627
#include "iceberg/table_metadata.h"
37-
#include "iceberg/table_requirement.h"
38-
#include "iceberg/table_requirements.h"
3928
#include "iceberg/table_update.h"
4029
#include "iceberg/test/matchers.h"
4130
#include "iceberg/test/mock_catalog.h"
42-
#include "iceberg/transaction.h"
43-
#include "iceberg/util/macros.h"
4431

4532
namespace iceberg {
4633
namespace {
47-
4834
using ::testing::ElementsAre;
4935
using ::testing::NiceMock;
5036

@@ -80,17 +66,17 @@ std::shared_ptr<Table> CreateTestTable(const TableIdentifier& identifier,
8066
return std::make_shared<Table>(identifier, metadata, "s3://bucket/table/metadata.json",
8167
nullptr, catalog);
8268
}
69+
} // namespace
8370

8471
TEST(TransactionPendingUpdateTest, CommitSetPropertiesUsesCatalog) {
8572
auto metadata = CreateBaseMetadata();
8673
const auto identifier = MakeIdentifier();
8774
auto catalog = std::make_shared<NiceMock<MockCatalog>>();
8875
auto table =
8976
CreateTestTable(identifier, std::make_shared<TableMetadata>(*metadata), catalog);
90-
BaseTransaction transaction(table, catalog);
91-
92-
auto update_properties = transaction.UpdateProperties();
93-
update_properties->Set("write.metadata.delete-after-commit.enabled", "true");
77+
auto transaction = table->NewTransaction();
78+
auto update_properties = transaction->UpdateProperties();
79+
update_properties->Set("new-key", "new-value");
9480

9581
EXPECT_CALL(*catalog,
9682
UpdateTable(::testing::Eq(identifier), ::testing::_, ::testing::_))
@@ -104,13 +90,13 @@ TEST(TransactionPendingUpdateTest, CommitSetPropertiesUsesCatalog) {
10490
dynamic_cast<const table::SetProperties*>(updates.front().get());
10591
EXPECT_NE(set_update, nullptr);
10692
const auto& updated = set_update->updated();
107-
auto it = updated.find("write.metadata.delete-after-commit.enabled");
93+
auto it = updated.find("new-key");
10894
EXPECT_NE(it, updated.end());
109-
EXPECT_EQ("true", it->second);
95+
EXPECT_EQ("new-value", it->second);
11096
return Result<std::unique_ptr<Table>>(std::unique_ptr<Table>());
11197
});
11298

113-
EXPECT_THAT(transaction.CommitTransaction(), IsOk());
99+
EXPECT_THAT(transaction->CommitTransaction(), IsOk());
114100
}
115101

116102
TEST(TransactionPendingUpdateTest, RemovePropertiesSkipsMissingKeys) {
@@ -119,9 +105,9 @@ TEST(TransactionPendingUpdateTest, RemovePropertiesSkipsMissingKeys) {
119105
auto catalog = std::make_shared<NiceMock<MockCatalog>>();
120106
auto table =
121107
CreateTestTable(identifier, std::make_shared<TableMetadata>(*metadata), catalog);
122-
BaseTransaction transaction(table, catalog);
108+
auto transaction = table->NewTransaction();
123109

124-
auto update_properties = transaction.UpdateProperties();
110+
auto update_properties = transaction->UpdateProperties();
125111
update_properties->Remove("missing").Remove("existing");
126112

127113
EXPECT_CALL(*catalog,
@@ -138,7 +124,7 @@ TEST(TransactionPendingUpdateTest, RemovePropertiesSkipsMissingKeys) {
138124
return Result<std::unique_ptr<Table>>(std::unique_ptr<Table>());
139125
});
140126

141-
EXPECT_THAT(transaction.CommitTransaction(), IsOk());
127+
EXPECT_THAT(transaction->CommitTransaction(), IsOk());
142128
}
143129

144130
TEST(TransactionPendingUpdateTest, AggregatesMultiplePendingUpdates) {
@@ -147,9 +133,9 @@ TEST(TransactionPendingUpdateTest, AggregatesMultiplePendingUpdates) {
147133
auto catalog = std::make_shared<NiceMock<MockCatalog>>();
148134
auto table =
149135
CreateTestTable(identifier, std::make_shared<TableMetadata>(*metadata), catalog);
150-
BaseTransaction transaction(table, catalog);
136+
auto transaction = table->NewTransaction();
151137

152-
auto update_properties = transaction.UpdateProperties();
138+
auto update_properties = transaction->UpdateProperties();
153139
update_properties->Set("new-key", "new-value").Remove("existing");
154140

155141
EXPECT_CALL(*catalog,
@@ -176,8 +162,7 @@ TEST(TransactionPendingUpdateTest, AggregatesMultiplePendingUpdates) {
176162
return Result<std::unique_ptr<Table>>(std::unique_ptr<Table>());
177163
});
178164

179-
EXPECT_THAT(transaction.CommitTransaction(), IsOk());
165+
EXPECT_THAT(transaction->CommitTransaction(), IsOk());
180166
}
181167

182-
} // namespace
183168
} // namespace iceberg

src/iceberg/transaction.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class ICEBERG_EXPORT Transaction {
3636
/// \brief Return the Table that this transaction will update
3737
///
3838
/// \return this transaction's table
39-
virtual const std::shared_ptr<Table>& table() const = 0;
39+
virtual const std::shared_ptr<const Table>& table() const = 0;
4040

4141
/// \brief Create a new update properties operation
4242
///

0 commit comments

Comments
 (0)