Skip to content

refactor: optimize Table accessors and TimePoint passing #414

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wgtmac merged 1 commit into from
Dec 17, 2025
Merged

refactor: optimize Table accessors and TimePoint passing #414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/iceberg/avro/avro_schema_util.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -734,7 +734,7 @@ Result<FieldProjection> ProjectMap(const MapType& map_type,
const auto& expected_key_field = map_type.key();
const auto& expected_value_field = map_type.value();

FieldProjection result, key_projection, value_projection;
FieldProjection result;
Copy link
Contributor

@HuaHuaY HuaHuaY Dec 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about adding readability-isolate-declaration in .clang-tidy? We can do this in next PR.

Copy link
Member Author

@wgtmac wgtmac Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

int32_t avro_key_id, avro_value_id;
::avro::NodePtr map_node;

Expand Down
6 changes: 3 additions & 3 deletions src/iceberg/table.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ Table::sort_orders() const {

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

const std::string& Table::metadata_file_location() const { return metadata_location_; }
std::string_view Table::metadata_file_location() const { return metadata_location_; }

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

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

Result<std::shared_ptr<Snapshot>> Table::current_snapshot() const {
return metadata_->Snapshot();
Expand Down
13 changes: 7 additions & 6 deletions src/iceberg/table.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,13 @@ class ICEBERG_EXPORT Table {
const TableProperties& properties() const;

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

/// \brief Return the table's base location
const std::string& location() const;
std::string_view location() const;

/// \brief Get the time when this table was last updated
///
/// \return the time when this table was last updated
const TimePointMs& last_updated_ms() const;
/// \brief Returns the time when this table was last updated
TimePointMs last_updated_ms() const;

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

/// \brief Returns the current metadata for this table
const std::shared_ptr<TableMetadata>& metadata() const;

private:
const TableIdentifier identifier_;
std::shared_ptr<TableMetadata> metadata_;
Expand Down
4 changes: 2 additions & 2 deletions src/iceberg/util/timepoint.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ Result<TimePointMs> TimePointMsFromUnixMs(int64_t unix_ms) {
return TimePointMs{std::chrono::milliseconds(unix_ms)};
}

int64_t UnixMsFromTimePointMs(const TimePointMs& time_point_ms) {
int64_t UnixMsFromTimePointMs(TimePointMs time_point_ms) {
return std::chrono::duration_cast<std::chrono::milliseconds>(
time_point_ms.time_since_epoch())
.count();
Expand All @@ -37,7 +37,7 @@ Result<TimePointNs> TimePointNsFromUnixNs(int64_t unix_ns) {
return TimePointNs{std::chrono::nanoseconds(unix_ns)};
}

int64_t UnixNsFromTimePointNs(const TimePointNs& time_point_ns) {
int64_t UnixNsFromTimePointNs(TimePointNs time_point_ns) {
return std::chrono::duration_cast<std::chrono::nanoseconds>(
time_point_ns.time_since_epoch())
.count();
Expand Down
4 changes: 2 additions & 2 deletions src/iceberg/util/timepoint.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ using TimePointNs =
ICEBERG_EXPORT Result<TimePointMs> TimePointMsFromUnixMs(int64_t unix_ms);

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

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

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

} // namespace iceberg
Loading