chore: refactor some obvious non optimal coding style (#440)

Author: zhjwpku

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;

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};

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.

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);

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 = {});

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;
   }

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();
 }
 

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;

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