Skip to content

Commit 25daf33

Browse files
authored
refactor: optimize Table accessors and TimePoint passing (#414)
- Update `Table::location()` and `Table::metadata_file_location()` to return `std::string_view` instead of `const std::string&` to avoid unnecessary reference indirection. - Change `TimePointMs` and `TimePointNs` arguments and return types to pass-by-value in `Table` and utility functions, as they are small types. - Expose `Table::metadata()` accessor to allow access to the underlying table metadata. - Clean up unused variable declarations in `avro_schema_util.cc`.
1 parent fc2cde0 commit 25daf33

File tree

5 files changed

+15
-14
lines changed

5 files changed

+15
-14
lines changed

src/iceberg/avro/avro_schema_util.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ Result<FieldProjection> ProjectMap(const MapType& map_type,
734734
const auto& expected_key_field = map_type.key();
735735
const auto& expected_value_field = map_type.value();
736736

737-
FieldProjection result, key_projection, value_projection;
737+
FieldProjection result;
738738
int32_t avro_key_id, avro_value_id;
739739
::avro::NodePtr map_node;
740740

src/iceberg/table.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,11 @@ Table::sort_orders() const {
8888

8989
const TableProperties& Table::properties() const { return metadata_->properties; }
9090

91-
const std::string& Table::metadata_file_location() const { return metadata_location_; }
91+
std::string_view Table::metadata_file_location() const { return metadata_location_; }
9292

93-
const std::string& Table::location() const { return metadata_->location; }
93+
std::string_view Table::location() const { return metadata_->location; }
9494

95-
const TimePointMs& Table::last_updated_ms() const { return metadata_->last_updated_ms; }
95+
TimePointMs Table::last_updated_ms() const { return metadata_->last_updated_ms; }
9696

9797
Result<std::shared_ptr<Snapshot>> Table::current_snapshot() const {
9898
return metadata_->Snapshot();

src/iceberg/table.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,15 +86,13 @@ class ICEBERG_EXPORT Table {
8686
const TableProperties& properties() const;
8787

8888
/// \brief Return the table's metadata file location
89-
const std::string& metadata_file_location() const;
89+
std::string_view metadata_file_location() const;
9090

9191
/// \brief Return the table's base location
92-
const std::string& location() const;
92+
std::string_view location() const;
9393

94-
/// \brief Get the time when this table was last updated
95-
///
96-
/// \return the time when this table was last updated
97-
const TimePointMs& last_updated_ms() const;
94+
/// \brief Returns the time when this table was last updated
95+
TimePointMs last_updated_ms() const;
9896

9997
/// \brief Return the table's current snapshot, return NotFoundError if not found
10098
Result<std::shared_ptr<Snapshot>> current_snapshot() const;
@@ -133,6 +131,9 @@ class ICEBERG_EXPORT Table {
133131
/// \brief Returns a FileIO to read and write table data and metadata files
134132
const std::shared_ptr<FileIO>& io() const;
135133

134+
/// \brief Returns the current metadata for this table
135+
const std::shared_ptr<TableMetadata>& metadata() const;
136+
136137
private:
137138
const TableIdentifier identifier_;
138139
std::shared_ptr<TableMetadata> metadata_;

src/iceberg/util/timepoint.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ Result<TimePointMs> TimePointMsFromUnixMs(int64_t unix_ms) {
2727
return TimePointMs{std::chrono::milliseconds(unix_ms)};
2828
}
2929

30-
int64_t UnixMsFromTimePointMs(const TimePointMs& time_point_ms) {
30+
int64_t UnixMsFromTimePointMs(TimePointMs time_point_ms) {
3131
return std::chrono::duration_cast<std::chrono::milliseconds>(
3232
time_point_ms.time_since_epoch())
3333
.count();
@@ -37,7 +37,7 @@ Result<TimePointNs> TimePointNsFromUnixNs(int64_t unix_ns) {
3737
return TimePointNs{std::chrono::nanoseconds(unix_ns)};
3838
}
3939

40-
int64_t UnixNsFromTimePointNs(const TimePointNs& time_point_ns) {
40+
int64_t UnixNsFromTimePointNs(TimePointNs time_point_ns) {
4141
return std::chrono::duration_cast<std::chrono::nanoseconds>(
4242
time_point_ns.time_since_epoch())
4343
.count();

src/iceberg/util/timepoint.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@ using TimePointNs =
3838
ICEBERG_EXPORT Result<TimePointMs> TimePointMsFromUnixMs(int64_t unix_ms);
3939

4040
/// \brief Returns a Unix timestamp in milliseconds from a TimePointMs
41-
ICEBERG_EXPORT int64_t UnixMsFromTimePointMs(const TimePointMs& time_point_ms);
41+
ICEBERG_EXPORT int64_t UnixMsFromTimePointMs(TimePointMs time_point_ms);
4242

4343
/// \brief Returns a TimePointNs from a Unix timestamp in nanoseconds
4444
ICEBERG_EXPORT Result<TimePointNs> TimePointNsFromUnixNs(int64_t unix_ns);
4545

4646
/// \brief Returns a Unix timestamp in nanoseconds from a TimePointNs
47-
ICEBERG_EXPORT int64_t UnixNsFromTimePointNs(const TimePointNs& time_point_ns);
47+
ICEBERG_EXPORT int64_t UnixNsFromTimePointNs(TimePointNs time_point_ns);
4848

4949
} // namespace iceberg

0 commit comments

Comments
 (0)