This is an automated email from the ASF dual-hosted git repository.
gangwu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new a6ff7533 chore: refactor some obvious non optimal coding style (#440)
a6ff7533 is described below
commit a6ff75338cb20038ae8b2e590707829e17e8fa8a
Author: Junwang Zhao <[email protected]>
AuthorDate: Fri Dec 26 13:19:42 2025 +0800
chore: refactor some obvious non optimal coding style (#440)
---
src/iceberg/json_internal.cc | 4 ++--
src/iceberg/manifest/manifest_reader.cc | 4 ++--
src/iceberg/manifest/manifest_reader.h | 3 ---
src/iceberg/schema.cc | 1 -
src/iceberg/schema.h | 3 +++
src/iceberg/table_metadata.cc | 8 ++++----
src/iceberg/util/snapshot_util.cc | 4 ++--
src/iceberg/util/timepoint.cc | 9 +++------
src/iceberg/util/timepoint.h | 2 +-
9 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/src/iceberg/json_internal.cc b/src/iceberg/json_internal.cc
index 82cb8ee7..9d955d46 100644
--- a/src/iceberg/json_internal.cc
+++ b/src/iceberg/json_internal.cc
@@ -787,9 +787,9 @@ nlohmann::json ToJson(const TableMetadata& table_metadata) {
// write properties map
json[kProperties] = table_metadata.properties.configs();
- if (std::ranges::find_if(table_metadata.snapshots, [&](const auto& snapshot)
{
+ if (std::ranges::any_of(table_metadata.snapshots, [&](const auto& snapshot) {
return snapshot->snapshot_id == table_metadata.current_snapshot_id;
- }) != table_metadata.snapshots.cend()) {
+ })) {
json[kCurrentSnapshotId] = table_metadata.current_snapshot_id;
} else {
json[kCurrentSnapshotId] = nlohmann::json::value_t::null;
diff --git a/src/iceberg/manifest/manifest_reader.cc
b/src/iceberg/manifest/manifest_reader.cc
index 2ba377fa..ba1ff860 100644
--- a/src/iceberg/manifest/manifest_reader.cc
+++ b/src/iceberg/manifest/manifest_reader.cc
@@ -570,7 +570,7 @@ bool RequireStatsProjection(const
std::shared_ptr<Expression>& row_filter,
return false;
}
const std::unordered_set<std::string_view> selected(columns.cbegin(),
columns.cend());
- if (selected.contains(ManifestReader::kAllColumns)) {
+ if (selected.contains(Schema::kAllColumns)) {
return false;
}
if (std::ranges::all_of(kStatsColumns, [&selected](const std::string& col) {
@@ -594,7 +594,7 @@ Result<std::shared_ptr<Schema>>
ProjectSchema(std::shared_ptr<Schema> schema,
std::vector<std::string> ManifestReader::WithStatsColumns(
const std::vector<std::string>& columns) {
- if (std::ranges::contains(columns, ManifestReader::kAllColumns)) {
+ if (std::ranges::contains(columns, Schema::kAllColumns)) {
return columns;
} else {
std::vector<std::string> updated_columns{columns};
diff --git a/src/iceberg/manifest/manifest_reader.h
b/src/iceberg/manifest/manifest_reader.h
index a35c1fb9..748d0e33 100644
--- a/src/iceberg/manifest/manifest_reader.h
+++ b/src/iceberg/manifest/manifest_reader.h
@@ -36,9 +36,6 @@ namespace iceberg {
/// \brief Read manifest entries from a manifest file.
class ICEBERG_EXPORT ManifestReader {
public:
- /// \brief Special value to select all columns from manifest files.
- static constexpr std::string_view kAllColumns = "*";
-
virtual ~ManifestReader() = default;
/// \brief Read all manifest entries in the manifest file.
diff --git a/src/iceberg/schema.cc b/src/iceberg/schema.cc
index 414219f8..4b4b6c26 100644
--- a/src/iceberg/schema.cc
+++ b/src/iceberg/schema.cc
@@ -163,7 +163,6 @@ Result<std::unique_ptr<StructLikeAccessor>>
Schema::GetAccessorById(
Result<std::unique_ptr<Schema>> Schema::Select(std::span<const std::string>
names,
bool case_sensitive) const {
- const std::string kAllColumns = "*";
if (std::ranges::find(names, kAllColumns) != names.end()) {
auto struct_type = ToStructType(*this);
return FromStructType(std::move(*struct_type), std::nullopt);
diff --git a/src/iceberg/schema.h b/src/iceberg/schema.h
index bb983962..410fa59e 100644
--- a/src/iceberg/schema.h
+++ b/src/iceberg/schema.h
@@ -48,6 +48,9 @@ class ICEBERG_EXPORT Schema : public StructType {
static constexpr int32_t kInitialSchemaId = 0;
static constexpr int32_t kInvalidColumnId = -1;
+ /// \brief Special value to select all columns from manifest files.
+ static constexpr std::string_view kAllColumns = "*";
+
explicit Schema(std::vector<SchemaField> fields,
std::optional<int32_t> schema_id = std::nullopt,
std::vector<int32_t> identifier_field_ids = {});
diff --git a/src/iceberg/table_metadata.cc b/src/iceberg/table_metadata.cc
index ba5b8f32..46b7f92d 100644
--- a/src/iceberg/table_metadata.cc
+++ b/src/iceberg/table_metadata.cc
@@ -548,12 +548,12 @@ Result<int32_t>
TableMetadataBuilder::Impl::AddSortOrder(const SortOrder& order)
// is now the last)
bool is_new_order =
last_added_order_id_.has_value() &&
- std::ranges::find_if(changes_, [new_order_id](const auto& change) {
+ std::ranges::any_of(changes_, [new_order_id](const auto& change) {
return change->kind() == TableUpdate::Kind::kAddSortOrder &&
internal::checked_cast<const table::AddSortOrder&>(*change)
.sort_order()
->order_id() == new_order_id;
- }) != changes_.cend();
+ });
last_added_order_id_ = is_new_order ? std::make_optional(new_order_id) :
std::nullopt;
return new_order_id;
}
@@ -613,12 +613,12 @@ Result<int32_t>
TableMetadataBuilder::Impl::AddPartitionSpec(const PartitionSpec
// is now the last)
bool is_new_spec =
last_added_spec_id_.has_value() &&
- std::ranges::find_if(changes_, [new_spec_id](const auto& change) {
+ std::ranges::any_of(changes_, [new_spec_id](const auto& change) {
return change->kind() == TableUpdate::Kind::kAddPartitionSpec &&
internal::checked_cast<const
table::AddPartitionSpec&>(*change)
.spec()
->spec_id() == new_spec_id;
- }) != changes_.cend();
+ });
last_added_spec_id_ = is_new_spec ? std::make_optional(new_spec_id) :
std::nullopt;
return new_spec_id;
}
diff --git a/src/iceberg/util/snapshot_util.cc
b/src/iceberg/util/snapshot_util.cc
index 1243a109..d2d083ea 100644
--- a/src/iceberg/util/snapshot_util.cc
+++ b/src/iceberg/util/snapshot_util.cc
@@ -124,7 +124,7 @@ Result<std::optional<std::shared_ptr<Snapshot>>>
SnapshotUtil::OldestAncestorAft
}
// the first ancestor after the given time can't be determined
- return NotFound("Cannot find snapshot older than {}",
FormatTimestamp(timestamp_ms));
+ return NotFound("Cannot find snapshot older than {}",
FormatTimePointMs(timestamp_ms));
}
Result<std::vector<int64_t>> SnapshotUtil::SnapshotIdsBetween(const Table&
table,
@@ -232,7 +232,7 @@ Result<int64_t> SnapshotUtil::SnapshotIdAsOfTime(const
Table& table,
TimePointMs timestamp_ms) {
auto snapshot_id = OptionalSnapshotIdAsOfTime(table, timestamp_ms);
ICEBERG_CHECK(snapshot_id.has_value(), "Cannot find a snapshot older than
{}",
- FormatTimestamp(timestamp_ms));
+ FormatTimePointMs(timestamp_ms));
return snapshot_id.value();
}
diff --git a/src/iceberg/util/timepoint.cc b/src/iceberg/util/timepoint.cc
index a8bc7708..0381e90a 100644
--- a/src/iceberg/util/timepoint.cc
+++ b/src/iceberg/util/timepoint.cc
@@ -45,12 +45,9 @@ int64_t UnixNsFromTimePointNs(TimePointNs time_point_ns) {
.count();
}
-std::string FormatTimestamp(TimePointMs time_point_ns) {
- // Convert TimePointMs to system_clock::time_point
- auto unix_ms = UnixMsFromTimePointMs(time_point_ns);
- auto time_point =
-
std::chrono::system_clock::time_point(std::chrono::milliseconds(unix_ms));
- auto time_t = std::chrono::system_clock::to_time_t(time_point);
+std::string FormatTimePointMs(TimePointMs time_point_ms) {
+ auto unix_ms = UnixMsFromTimePointMs(time_point_ms);
+ auto time_t = std::chrono::system_clock::to_time_t(time_point_ms);
// Format as ISO 8601-like string: YYYY-MM-DD HH:MM:SS
std::ostringstream oss;
diff --git a/src/iceberg/util/timepoint.h b/src/iceberg/util/timepoint.h
index 48e630ae..6052c94a 100644
--- a/src/iceberg/util/timepoint.h
+++ b/src/iceberg/util/timepoint.h
@@ -47,6 +47,6 @@ ICEBERG_EXPORT Result<TimePointNs>
TimePointNsFromUnixNs(int64_t unix_ns);
ICEBERG_EXPORT int64_t UnixNsFromTimePointNs(TimePointNs time_point_ns);
/// \brief Returns a human-readable string representation of a TimePointMs
-ICEBERG_EXPORT std::string FormatTimestamp(TimePointMs time_point_ns);
+ICEBERG_EXPORT std::string FormatTimePointMs(TimePointMs time_point_ms);
} // namespace iceberg
