Skip to content

Commit f869377

Browse files
committed
fix ubsan
1 parent 5c703d9 commit f869377

4 files changed

Lines changed: 27 additions & 21 deletions

File tree

src/iceberg/transaction.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,11 @@ const TableMetadata* Transaction::base() const { return metadata_builder_->base(
5151

5252
const TableMetadata* Transaction::current() const { return metadata_builder_->current(); }
5353

54-
Status Transaction::AddUpdate(std::shared_ptr<PendingUpdate> update) {
54+
Status Transaction::AddUpdate(const std::shared_ptr<PendingUpdate>& update) {
5555
if (!last_update_committed_) {
5656
return InvalidArgument("Cannot add update when previous update is not committed");
5757
}
58-
pending_updates_.emplace_back(std::move(update));
58+
pending_updates_.emplace_back(std::weak_ptr<PendingUpdate>(update));
5959
last_update_committed_ = false;
6060
return {};
6161
}

src/iceberg/transaction.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ class ICEBERG_EXPORT Transaction : public std::enable_shared_from_this<Transacti
6363
private:
6464
Transaction(std::shared_ptr<Table> table, Kind kind, bool auto_commit);
6565

66-
Status AddUpdate(std::shared_ptr<PendingUpdate> update);
66+
Status AddUpdate(const std::shared_ptr<PendingUpdate>& update);
6767

6868
/// \brief Apply the pending changes to current table.
6969
Status Apply(std::vector<std::unique_ptr<TableUpdate>> updates);
@@ -75,9 +75,11 @@ class ICEBERG_EXPORT Transaction : public std::enable_shared_from_this<Transacti
7575
const Kind kind_;
7676
const bool auto_commit_;
7777

78-
// Staged updates
78+
// To make the state simple, we require updates are added and committed in order.
7979
bool last_update_committed_ = true;
80-
std::vector<std::shared_ptr<PendingUpdate>> pending_updates_;
80+
// Staged updates. Use weak_ptr to avoid circular references.
81+
std::vector<std::weak_ptr<PendingUpdate>> pending_updates_;
82+
// Accumulated updates from all pending updates.
8183
std::unique_ptr<TableMetadataBuilder> metadata_builder_;
8284
};
8385

src/iceberg/update/update_properties.cc

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919

2020
#include "iceberg/update/update_properties.h"
2121

22+
#include <charconv>
2223
#include <cstdint>
2324
#include <memory>
25+
#include <system_error>
2426

2527
#include "iceberg/metrics_config.h"
2628
#include "iceberg/result.h"
@@ -32,12 +34,12 @@
3234

3335
namespace iceberg {
3436

35-
Result<std::unique_ptr<UpdateProperties>> UpdateProperties::Make(
37+
Result<std::shared_ptr<UpdateProperties>> UpdateProperties::Make(
3638
std::shared_ptr<Transaction> transaction) {
3739
if (!transaction) [[unlikely]] {
3840
return InvalidArgument("Cannot create UpdateProperties without a transaction");
3941
}
40-
return std::unique_ptr<UpdateProperties>(new UpdateProperties(std::move(transaction)));
42+
return std::shared_ptr<UpdateProperties>(new UpdateProperties(std::move(transaction)));
4143
}
4244

4345
UpdateProperties::UpdateProperties(std::shared_ptr<Transaction> transaction)
@@ -83,21 +85,23 @@ Result<PendingUpdate::ApplyResult> UpdateProperties::Apply() {
8385

8486
auto iter = updates_.find(TableProperties::kFormatVersion.key());
8587
if (iter != updates_.end()) {
86-
try {
87-
int parsed_version = std::stoi(iter->second);
88-
if (parsed_version > TableMetadata::kSupportedTableFormatVersion) {
89-
return InvalidArgument(
90-
"Cannot upgrade table to unsupported format version: v{} (supported: v{})",
91-
parsed_version, TableMetadata::kSupportedTableFormatVersion);
92-
}
93-
format_version_ = static_cast<int8_t>(parsed_version);
94-
} catch (const std::invalid_argument& e) {
95-
return InvalidArgument("Invalid format version '{}': not a valid integer",
96-
iter->second);
97-
} catch (const std::out_of_range& e) {
98-
return InvalidArgument("Format version '{}' is out of range", iter->second);
88+
int parsed_version = 0;
89+
const auto& val = iter->second;
90+
auto [ptr, ec] = std::from_chars(val.data(), val.data() + val.size(), parsed_version);
91+
92+
if (ec == std::errc::invalid_argument) {
93+
return InvalidArgument("Invalid format version '{}': not a valid integer", val);
94+
} else if (ec == std::errc::result_out_of_range) {
95+
return InvalidArgument("Format version '{}' is out of range", val);
9996
}
10097

98+
if (parsed_version > TableMetadata::kSupportedTableFormatVersion) {
99+
return InvalidArgument(
100+
"Cannot upgrade table to unsupported format version: v{} (supported: v{})",
101+
parsed_version, TableMetadata::kSupportedTableFormatVersion);
102+
}
103+
format_version_ = static_cast<int8_t>(parsed_version);
104+
101105
updates_.erase(iter);
102106
}
103107

src/iceberg/update/update_properties.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ namespace iceberg {
3434
/// \brief Updates table properties.
3535
class ICEBERG_EXPORT UpdateProperties : public PendingUpdate {
3636
public:
37-
static Result<std::unique_ptr<UpdateProperties>> Make(
37+
static Result<std::shared_ptr<UpdateProperties>> Make(
3838
std::shared_ptr<Transaction> transaction);
3939

4040
~UpdateProperties() override;

0 commit comments

Comments
 (0)