This is an automated email from the ASF dual-hosted git repository.
xuanwo 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 e7509f2 refactor: use ICEBERG_CHECK where possible (#244)
e7509f2 is described below
commit e7509f2602414e9a807779f6ddee190272458784
Author: Junwang Zhao <zhjw...@gmail.com>
AuthorDate: Wed Sep 24 16:15:41 2025 +0800
refactor: use ICEBERG_CHECK where possible (#244)
This will simplify the code and potentially optimize the binary due to
the [[unlikely]] attribute in ICEBERG_CHECK.
---
src/iceberg/avro/avro_stream_internal.cc | 25 +++++++------------------
src/iceberg/type.cc | 31 ++++++++++++-------------------
src/iceberg/util/decimal.cc | 6 ++----
3 files changed, 21 insertions(+), 41 deletions(-)
diff --git a/src/iceberg/avro/avro_stream_internal.cc
b/src/iceberg/avro/avro_stream_internal.cc
index 3ff64bb..f299b52 100644
--- a/src/iceberg/avro/avro_stream_internal.cc
+++ b/src/iceberg/avro/avro_stream_internal.cc
@@ -66,10 +66,8 @@ bool AvroInputStream::next(const uint8_t** data, size_t*
len) {
}
void AvroInputStream::backup(size_t len) {
- if (len > buffer_pos_) {
- throw IcebergError(
- std::format("Cannot backup {} bytes, only {} bytes available", len,
buffer_pos_));
- }
+ ICEBERG_CHECK(len <= buffer_pos_, "Cannot backup {} bytes, only {} bytes
available",
+ len, buffer_pos_);
buffer_pos_ -= len;
byte_count_ -= len;
@@ -90,10 +88,7 @@ size_t AvroInputStream::byteCount() const { return
byte_count_; }
void AvroInputStream::seek(int64_t position) {
auto status = input_stream_->Seek(position);
- if (!status.ok()) {
- throw IcebergError(
- std::format("Failed to seek to {}, got {}", position,
status.ToString()));
- }
+ ICEBERG_CHECK(status.ok(), "Failed to seek to {}, got {}", position,
status.ToString());
buffer_pos_ = 0;
available_bytes_ = 0;
@@ -121,10 +116,8 @@ bool AvroOutputStream::next(uint8_t** data, size_t* len) {
}
void AvroOutputStream::backup(size_t len) {
- if (len > buffer_pos_) {
- throw IcebergError(
- std::format("Cannot backup {} bytes, only {} bytes available", len,
buffer_pos_));
- }
+ ICEBERG_CHECK(len <= buffer_pos_, "Cannot backup {} bytes, only {} bytes
available",
+ len, buffer_pos_);
buffer_pos_ -= len;
}
@@ -133,16 +126,12 @@ uint64_t AvroOutputStream::byteCount() const { return
flushed_bytes_ + buffer_po
void AvroOutputStream::flush() {
if (buffer_pos_ > 0) {
auto status = output_stream_->Write(buffer_.data(), buffer_pos_);
- if (!status.ok()) {
- throw IcebergError(std::format("Write failed {}", status.ToString()));
- }
+ ICEBERG_CHECK(status.ok(), "Write failed {}", status.ToString());
flushed_bytes_ += buffer_pos_;
buffer_pos_ = 0;
}
auto status = output_stream_->Flush();
- if (!status.ok()) {
- throw IcebergError(std::format("Flush failed {}", status.ToString()));
- }
+ ICEBERG_CHECK(status.ok(), "Flush failed {}", status.ToString());
}
const std::shared_ptr<::arrow::io::OutputStream>&
AvroOutputStream::arrow_output_stream()
diff --git a/src/iceberg/type.cc b/src/iceberg/type.cc
index 8d230d7..7b0f094 100644
--- a/src/iceberg/type.cc
+++ b/src/iceberg/type.cc
@@ -133,10 +133,9 @@ Status StructType::InitFieldByLowerCaseName() const {
}
ListType::ListType(SchemaField element) : element_(std::move(element)) {
- if (element_.name() != kElementName) {
- throw IcebergError(std::format("ListType: child field name should be '{}',
was '{}'",
- kElementName, element_.name()));
- }
+ ICEBERG_CHECK(element_.name() == kElementName,
+ "ListType: child field name should be '{}', was '{}'",
kElementName,
+ element_.name());
}
ListType::ListType(int32_t field_id, std::shared_ptr<Type> type, bool optional)
@@ -189,14 +188,12 @@ bool ListType::Equals(const Type& other) const {
MapType::MapType(SchemaField key, SchemaField value)
: fields_{std::move(key), std::move(value)} {
- if (this->key().name() != kKeyName) {
- throw IcebergError(std::format("MapType: key field name should be '{}',
was '{}'",
- kKeyName, this->key().name()));
- }
- if (this->value().name() != kValueName) {
- throw IcebergError(std::format("MapType: value field name should be '{}',
was '{}'",
- kValueName, this->value().name()));
- }
+ ICEBERG_CHECK(this->key().name() == kKeyName,
+ "MapType: key field name should be '{}', was '{}'", kKeyName,
+ this->key().name());
+ ICEBERG_CHECK(this->value().name() == kValueName,
+ "MapType: value field name should be '{}', was '{}'",
kValueName,
+ this->value().name());
}
const SchemaField& MapType::key() const { return fields_[0]; }
@@ -278,10 +275,8 @@ bool DoubleType::Equals(const Type& other) const { return
other.type_id() == kTy
DecimalType::DecimalType(int32_t precision, int32_t scale)
: precision_(precision), scale_(scale) {
- if (precision < 0 || precision > kMaxPrecision) {
- throw IcebergError(
- std::format("DecimalType: precision must be in [0, 38], was {}",
precision));
- }
+ ICEBERG_CHECK(precision >= 0 && precision <= kMaxPrecision,
+ "DecimalType: precision must be in [0, 38], was {}",
precision);
}
int32_t DecimalType::precision() const { return precision_; }
@@ -329,9 +324,7 @@ std::string UuidType::ToString() const { return "uuid"; }
bool UuidType::Equals(const Type& other) const { return other.type_id() ==
kTypeId; }
FixedType::FixedType(int32_t length) : length_(length) {
- if (length < 0) {
- throw IcebergError(std::format("FixedType: length must be >= 0, was {}",
length));
- }
+ ICEBERG_CHECK(length >= 0, "FixedType: length must be >= 0, was {}", length);
}
int32_t FixedType::length() const { return length_; }
diff --git a/src/iceberg/util/decimal.cc b/src/iceberg/util/decimal.cc
index 606bf2f..5018574 100644
--- a/src/iceberg/util/decimal.cc
+++ b/src/iceberg/util/decimal.cc
@@ -283,10 +283,8 @@ bool RescaleWouldCauseDataLoss(const Decimal& value,
int32_t delta_scale,
Decimal::Decimal(std::string_view str) {
auto result = Decimal::FromString(str);
- if (!result) {
- throw IcebergError(std::format("Failed to parse Decimal from string: {},
error: {}",
- str, result.error().message));
- }
+ ICEBERG_CHECK(result, "Failed to parse Decimal from string: {}, error: {}",
str,
+ result.error().message);
*this = std::move(result.value());
}
