Skip to content

Commit ff3a3f9

Browse files
wgtmacwgtmac
committed
refactor: unify lazy init for table
1 parent 52b9c5e commit ff3a3f9

4 files changed

Lines changed: 93 additions & 51 deletions

File tree

‎src/iceberg/table.cc‎

Lines changed: 43 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -58,59 +58,71 @@ Status Table::Refresh() {
5858
io_ = std::move(refreshed_table->io_);
5959
properties_ = std::move(refreshed_table->properties_);
6060

61-
schemas_map_.reset();
62-
partition_spec_map_.reset();
63-
sort_orders_map_.reset();
61+
// Reset lazy-initialized maps using move assignment
62+
schemas_map_ = Lazy<InitSchemasMap>{};
63+
partition_spec_map_ = Lazy<InitPartitionSpecsMap>{};
64+
sort_orders_map_ = Lazy<InitSortOrdersMap>{};
6465
}
6566
return {};
6667
}
6768

6869
Result<std::shared_ptr<Schema>> Table::schema() const { return metadata_->Schema(); }
6970

70-
const std::shared_ptr<std::unordered_map<int32_t, std::shared_ptr<Schema>>>&
71-
Table::schemas() const {
72-
if (!schemas_map_) {
73-
schemas_map_ =
74-
std::make_shared<std::unordered_map<int32_t, std::shared_ptr<Schema>>>();
75-
for (const auto& schema : metadata_->schemas) {
76-
if (schema->schema_id()) {
77-
schemas_map_->emplace(schema->schema_id().value(), schema);
78-
}
71+
Result<std::unordered_map<int32_t, std::shared_ptr<Schema>>> Table::InitSchemasMap(
72+
const Table& self) {
73+
std::unordered_map<int32_t, std::shared_ptr<Schema>> schemas_map;
74+
for (const auto& schema : self.metadata_->schemas) {
75+
if (schema->schema_id()) {
76+
schemas_map.emplace(schema->schema_id().value(), schema);
7977
}
8078
}
81-
return schemas_map_;
79+
return schemas_map;
80+
}
81+
82+
Result<std::reference_wrapper<const std::unordered_map<int32_t, std::shared_ptr<Schema>>>>
83+
Table::schemas() const {
84+
ICEBERG_ASSIGN_OR_RAISE(auto schemas_ref, schemas_map_.Get(*this));
85+
return std::cref(schemas_ref.get());
8286
}
8387

8488
Result<std::shared_ptr<PartitionSpec>> Table::spec() const {
8589
return metadata_->PartitionSpec();
8690
}
8791

88-
const std::shared_ptr<std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>>&
89-
Table::specs() const {
90-
if (!partition_spec_map_) {
91-
partition_spec_map_ =
92-
std::make_shared<std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>>();
93-
for (const auto& spec : metadata_->partition_specs) {
94-
partition_spec_map_->emplace(spec->spec_id(), spec);
95-
}
92+
Result<std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>>
93+
Table::InitPartitionSpecsMap(const Table& self) {
94+
std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>> partition_spec_map;
95+
for (const auto& spec : self.metadata_->partition_specs) {
96+
partition_spec_map.emplace(spec->spec_id(), spec);
9697
}
97-
return partition_spec_map_;
98+
return partition_spec_map;
99+
}
100+
101+
Result<std::reference_wrapper<
102+
const std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>>>
103+
Table::specs() const {
104+
ICEBERG_ASSIGN_OR_RAISE(auto specs_ref, partition_spec_map_.Get(*this));
105+
return std::cref(specs_ref.get());
98106
}
99107

100108
Result<std::shared_ptr<SortOrder>> Table::sort_order() const {
101109
return metadata_->SortOrder();
102110
}
103111

104-
const std::shared_ptr<std::unordered_map<int32_t, std::shared_ptr<SortOrder>>>&
105-
Table::sort_orders() const {
106-
if (!sort_orders_map_) {
107-
sort_orders_map_ =
108-
std::make_shared<std::unordered_map<int32_t, std::shared_ptr<SortOrder>>>();
109-
for (const auto& order : metadata_->sort_orders) {
110-
sort_orders_map_->emplace(order->order_id(), order);
111-
}
112+
Result<std::unordered_map<int32_t, std::shared_ptr<SortOrder>>> Table::InitSortOrdersMap(
113+
const Table& self) {
114+
std::unordered_map<int32_t, std::shared_ptr<SortOrder>> sort_orders_map;
115+
for (const auto& order : self.metadata_->sort_orders) {
116+
sort_orders_map.emplace(order->order_id(), order);
112117
}
113-
return sort_orders_map_;
118+
return sort_orders_map;
119+
}
120+
121+
Result<
122+
std::reference_wrapper<const std::unordered_map<int32_t, std::shared_ptr<SortOrder>>>>
123+
Table::sort_orders() const {
124+
ICEBERG_ASSIGN_OR_RAISE(auto sort_orders_ref, sort_orders_map_.Get(*this));
125+
return std::cref(sort_orders_ref.get());
114126
}
115127

116128
const TableProperties& Table::properties() const { return *properties_; }

‎src/iceberg/table.h‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include "iceberg/snapshot.h"
2929
#include "iceberg/table_identifier.h"
3030
#include "iceberg/type_fwd.h"
31+
#include "iceberg/util/lazy.h"
3132

3233
namespace iceberg {
3334

@@ -60,24 +61,24 @@ class ICEBERG_EXPORT Table {
6061
Result<std::shared_ptr<Schema>> schema() const;
6162

6263
/// \brief Return a map of schema for this table
63-
/// \note This method is **not** thread-safe in the current implementation.
64-
const std::shared_ptr<std::unordered_map<int32_t, std::shared_ptr<Schema>>>& schemas()
65-
const;
64+
Result<
65+
std::reference_wrapper<const std::unordered_map<int32_t, std::shared_ptr<Schema>>>>
66+
schemas() const;
6667

6768
/// \brief Return the partition spec for this table, return NotFoundError if not found
6869
Result<std::shared_ptr<PartitionSpec>> spec() const;
6970

7071
/// \brief Return a map of partition specs for this table
71-
/// \note This method is **not** thread-safe in the current implementation.
72-
const std::shared_ptr<std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>>&
72+
Result<std::reference_wrapper<
73+
const std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>>>
7374
specs() const;
7475

7576
/// \brief Return the sort order for this table, return NotFoundError if not found
7677
Result<std::shared_ptr<SortOrder>> sort_order() const;
7778

7879
/// \brief Return a map of sort order IDs to sort orders for this table
79-
/// \note This method is **not** thread-safe in the current implementation.
80-
const std::shared_ptr<std::unordered_map<int32_t, std::shared_ptr<SortOrder>>>&
80+
Result<std::reference_wrapper<
81+
const std::unordered_map<int32_t, std::shared_ptr<SortOrder>>>>
8182
sort_orders() const;
8283

8384
/// \brief Return a map of string properties for this table
@@ -118,20 +119,24 @@ class ICEBERG_EXPORT Table {
118119
const std::shared_ptr<FileIO>& io() const;
119120

120121
private:
122+
static Result<std::unordered_map<int32_t, std::shared_ptr<Schema>>> InitSchemasMap(
123+
const Table&);
124+
static Result<std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>>
125+
InitPartitionSpecsMap(const Table&);
126+
static Result<std::unordered_map<int32_t, std::shared_ptr<SortOrder>>>
127+
InitSortOrdersMap(const Table&);
128+
121129
const TableIdentifier identifier_;
122130
std::shared_ptr<TableMetadata> metadata_;
123131
std::string metadata_location_;
124132
std::shared_ptr<FileIO> io_;
125133
std::shared_ptr<Catalog> catalog_;
126134
std::unique_ptr<TableProperties> properties_;
127135

128-
// Cache lazy-initialized maps.
129-
mutable std::shared_ptr<std::unordered_map<int32_t, std::shared_ptr<Schema>>>
130-
schemas_map_;
131-
mutable std::shared_ptr<std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>>
132-
partition_spec_map_;
133-
mutable std::shared_ptr<std::unordered_map<int32_t, std::shared_ptr<SortOrder>>>
134-
sort_orders_map_;
136+
// Lazy-initialized maps.
137+
Lazy<InitSchemasMap> schemas_map_;
138+
Lazy<InitPartitionSpecsMap> partition_spec_map_;
139+
Lazy<InitSortOrdersMap> sort_orders_map_;
135140
};
136141

137142
} // namespace iceberg

‎src/iceberg/test/table_test.cc‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,22 @@ TEST(Table, TableV1) {
4949
ASSERT_TRUE(schema.has_value());
5050
ASSERT_EQ(schema.value()->fields().size(), 3);
5151
auto schemas = table.schemas();
52-
ASSERT_TRUE(schemas->empty());
52+
ASSERT_TRUE(schemas.has_value());
53+
ASSERT_TRUE(schemas.value().get().empty());
5354

5455
// Check table spec
5556
auto spec = table.spec();
5657
ASSERT_TRUE(spec.has_value());
5758
auto specs = table.specs();
58-
ASSERT_EQ(1UL, specs->size());
59+
ASSERT_TRUE(specs.has_value());
60+
ASSERT_EQ(1UL, specs.value().get().size());
5961

6062
// Check table sort_order
6163
auto sort_order = table.sort_order();
6264
ASSERT_TRUE(sort_order.has_value());
6365
auto sort_orders = table.sort_orders();
64-
ASSERT_EQ(1UL, sort_orders->size());
66+
ASSERT_TRUE(sort_orders.has_value());
67+
ASSERT_EQ(1UL, sort_orders.value().get().size());
6568

6669
// Check table location
6770
auto location = table.location();
@@ -89,19 +92,22 @@ TEST(Table, TableV2) {
8992
ASSERT_TRUE(schema.has_value());
9093
ASSERT_EQ(schema.value()->fields().size(), 3);
9194
auto schemas = table.schemas();
92-
ASSERT_FALSE(schemas->empty());
95+
ASSERT_TRUE(schemas.has_value());
96+
ASSERT_FALSE(schemas.value().get().empty());
9397

9498
// Check partition spec
9599
auto spec = table.spec();
96100
ASSERT_TRUE(spec.has_value());
97101
auto specs = table.specs();
98-
ASSERT_EQ(1UL, specs->size());
102+
ASSERT_TRUE(specs.has_value());
103+
ASSERT_EQ(1UL, specs.value().get().size());
99104

100105
// Check sort order
101106
auto sort_order = table.sort_order();
102107
ASSERT_TRUE(sort_order.has_value());
103108
auto sort_orders = table.sort_orders();
104-
ASSERT_EQ(1UL, sort_orders->size());
109+
ASSERT_TRUE(sort_orders.has_value());
110+
ASSERT_EQ(1UL, sort_orders.value().get().size());
105111

106112
// Check table location
107113
auto location = table.location();

‎src/iceberg/util/lazy.h‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,25 @@ class Lazy {
4444
using T = Trait<decltype(InitFunc)>::ReturnType;
4545

4646
public:
47+
Lazy() = default;
48+
49+
// Move assignment operator to support resetting the lazy state
50+
Lazy& operator=(Lazy&& other) noexcept {
51+
if (this != &other) {
52+
// Destroy and reconstruct to reset the once_flag
53+
this->~Lazy();
54+
new (this) Lazy();
55+
}
56+
return *this;
57+
}
58+
59+
// Delete copy operations since std::once_flag is not copyable
60+
Lazy(const Lazy&) = delete;
61+
Lazy& operator=(const Lazy&) = delete;
62+
63+
// Move constructor
64+
Lazy(Lazy&& other) noexcept : Lazy() {}
65+
4766
template <typename... Args>
4867
requires std::invocable<decltype(InitFunc), Args...> &&
4968
std::same_as<std::invoke_result_t<decltype(InitFunc), Args...>, Result<T>>

0 commit comments

Comments
 (0)