Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
41 changes: 41 additions & 0 deletions docs/code-style.md
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,47 @@ virtual ~ClassName() = default;
- Use `std::optional<T>` for optional values.
- Use `ScopeGuard` (in `src/paimon/common/utils/`) for RAII cleanup.

### Checked Class-Pointer Casts

Include `paimon/common/utils/checked_cast.h` and use the Paimon cast helpers for conversions
between class pointers:

- Use `checked_pointer_cast<T>` instead of `std::static_pointer_cast<T>` for `std::shared_ptr`
and `std::unique_ptr` conversions.
- Use `checked_cast<T*>` instead of `static_cast<T*>` for raw-pointer casts within a polymorphic
class hierarchy.
- Do not call `arrow::internal::checked_cast` or
`arrow::internal::checked_pointer_cast` directly.

```cpp
#include "paimon/common/utils/checked_cast.h"

std::shared_ptr<arrow::StructArray> struct_array =
checked_pointer_cast<arrow::StructArray>(array);
arrow::StringBuilder* string_builder = checked_cast<arrow::StringBuilder*>(builder);
```

The helpers use Arrow's debug-checked implementation: casts are dynamic in debug builds and
static in release builds. They therefore express an internal type invariant; they do not validate
recoverable runtime input. For data originating from files, schemas, C Data Interface imports, or
other external boundaries, check for null and validate the Arrow type before casting:

```cpp
if (!array || array->type_id() != arrow::Type::STRUCT) {
return Status::Invalid("expected a struct array");
}
auto struct_array = checked_pointer_cast<arrow::StructArray>(array);
```

Do not add a null check after `checked_pointer_cast` as a substitute for runtime type validation;
such a check detects a mismatched type only in debug builds. Use `dynamic_cast` and check its
result when cast failure is an expected runtime branch and no explicit type discriminator is
available.

These helpers do not apply to non-polymorphic pointer conversions such as `void*` callback
contexts, raw byte buffers, C FFI handles, or other opaque storage. Keep the appropriate explicit
cast for those cases.

---

## Comments & Documentation
Expand Down
6 changes: 3 additions & 3 deletions src/paimon/common/data/binary_array_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@
#include "arrow/api.h"
#include "arrow/array/array_nested.h"
#include "arrow/ipc/json_simple.h"
#include "arrow/util/checked_cast.h"
#include "gtest/gtest.h"
#include "paimon/common/data/binary_array_writer.h"
#include "paimon/common/data/binary_map.h"
#include "paimon/common/data/columnar/columnar_array.h"
#include "paimon/common/utils/checked_cast.h"
#include "paimon/common/utils/date_time_utils.h"
#include "paimon/testing/utils/binary_row_generator.h"
#include "paimon/testing/utils/testharness.h"
Expand Down Expand Up @@ -334,7 +334,7 @@ TEST(BinaryArrayTest, TestFromLongArray) {
auto f1 = arrow::ipc::internal::json::ArrayFromJSON(arrow::list(arrow::int64()),
R"([[123, null], [789], [12345], [12]])")
.ValueOrDie();
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
auto list_array = checked_pointer_cast<arrow::ListArray>(f1);
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/0, 2);

BinaryArray ret = BinaryArray::FromLongArray(&array, pool.get());
Expand Down Expand Up @@ -365,7 +365,7 @@ TEST(BinaryArrayTest, TestFromAllNullLongArray) {
auto f1 = arrow::ipc::internal::json::ArrayFromJSON(arrow::list(arrow::int64()),
R"([[null, null], [789], [12345], [12]])")
.ValueOrDie();
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
auto list_array = checked_pointer_cast<arrow::ListArray>(f1);
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/0, 2);

BinaryArray ret = BinaryArray::FromLongArray(&array, pool.get());
Expand Down
9 changes: 3 additions & 6 deletions src/paimon/common/data/binary_row_writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
#include <utility>

#include "arrow/type.h"
#include "arrow/util/checked_cast.h"
#include "fmt/format.h"
#include "paimon/common/data/binary_string.h"
#include "paimon/common/utils/checked_cast.h"
#include "paimon/common/utils/date_time_utils.h"
#include "paimon/data/decimal.h"
#include "paimon/data/timestamp.h"
Expand Down Expand Up @@ -125,8 +125,7 @@ Result<BinaryRowWriter::FieldSetterFunc> BinaryRowWriter::CreateFieldSetter(
break;
}
case arrow::Type::type::TIMESTAMP: {
auto timestamp_type =
arrow::internal::checked_pointer_cast<arrow::TimestampType>(field_type);
auto timestamp_type = checked_pointer_cast<arrow::TimestampType>(field_type);
int32_t precision = DateTimeUtils::GetPrecisionFromType(timestamp_type);
field_setter = [field_idx, precision](const VariantType& field,
BinaryRowWriter* writer) -> void {
Expand All @@ -144,9 +143,7 @@ Result<BinaryRowWriter::FieldSetterFunc> BinaryRowWriter::CreateFieldSetter(
return field_setter;
}
case arrow::Type::type::DECIMAL128: {
auto* decimal_type =
arrow::internal::checked_cast<arrow::Decimal128Type*>(field_type.get());
assert(decimal_type);
auto* decimal_type = checked_cast<arrow::Decimal128Type*>(field_type.get());
auto precision = decimal_type->precision();
auto scale = decimal_type->scale();
field_setter = [field_idx, precision, scale](const VariantType& field,
Expand Down
8 changes: 4 additions & 4 deletions src/paimon/common/data/blob_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "paimon/common/data/blob_view_struct.h"
#include "paimon/common/types/data_field.h"
#include "paimon/common/utils/arrow/status_utils.h"
#include "paimon/common/utils/checked_cast.h"
#include "paimon/common/utils/string_utils.h"
namespace arrow {
class Array;
Expand Down Expand Up @@ -62,7 +63,7 @@ Result<BlobUtils::SeparatedStructArrays> BlobUtils::SeparateBlobArray(
const std::shared_ptr<arrow::StructArray>& struct_array,
const std::set<std::string>& inline_fields) {
std::shared_ptr<arrow::StructType> old_type =
std::static_pointer_cast<arrow::StructType>(struct_array->type());
checked_pointer_cast<arrow::StructType>(struct_array->type());
const auto& old_fields = old_type->fields();
const auto& old_arrays = struct_array->fields();

Expand Down Expand Up @@ -146,12 +147,11 @@ Status BlobUtils::ValidateBlobInlineFields(const std::shared_ptr<arrow::StructAr
if (!field_array) {
continue;
}
const auto* binary_array =
arrow::internal::checked_cast<const arrow::LargeBinaryArray*>(field_array.get());
if (!binary_array) {
if (field_array->type_id() != arrow::Type::LARGE_BINARY) {
return Status::Invalid(
fmt::format("cannot cast array for field {} to LargeBinaryArray", field_name));
}
const auto* binary_array = checked_cast<const arrow::LargeBinaryArray*>(field_array.get());
for (int64_t row = 0; row < binary_array->length(); ++row) {
if (binary_array->IsNull(row)) {
continue;
Expand Down
5 changes: 3 additions & 2 deletions src/paimon/common/data/blob_utils_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "paimon/common/data/blob_descriptor.h"
#include "paimon/common/data/blob_view_struct.h"
#include "paimon/common/types/data_field.h"
#include "paimon/common/utils/checked_cast.h"
#include "paimon/data/blob.h"
#include "paimon/memory/memory_pool.h"
#include "paimon/testing/utils/testharness.h"
Expand Down Expand Up @@ -164,7 +165,7 @@ TEST_F(BlobUtilsTest, SeparateBlobArray) {
.ValueOrDie();

std::shared_ptr<arrow::StructArray> struct_array =
std::static_pointer_cast<arrow::StructArray>(raw_struct_array);
checked_pointer_cast<arrow::StructArray>(raw_struct_array);

ASSERT_OK_AND_ASSIGN(auto separated,
BlobUtils::SeparateBlobArray(struct_array, /*inline_fields=*/{}));
Expand Down Expand Up @@ -217,7 +218,7 @@ TEST_F(BlobUtilsTest, SeparateBlobArrayWithPartialInline) {
auto raw_struct_array =
arrow::StructArray::Make({int_array, blob_array_1, blob_array_2}, schema->fields())
.ValueOrDie();
auto struct_array = std::static_pointer_cast<arrow::StructArray>(raw_struct_array);
auto struct_array = checked_pointer_cast<arrow::StructArray>(raw_struct_array);

// f2_blob_1 is inline, f3_blob_2 goes to blob
ASSERT_OK_AND_ASSIGN(auto separated, BlobUtils::SeparateBlobArray(
Expand Down
20 changes: 7 additions & 13 deletions src/paimon/common/data/columnar/columnar_array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@
#include "arrow/array/array_nested.h"
#include "arrow/array/array_primitive.h"
#include "arrow/type_traits.h"
#include "arrow/util/checked_cast.h"
#include "arrow/util/decimal.h"
#include "fmt/format.h"
#include "paimon/common/data/columnar/columnar_batch_context.h"
#include "paimon/common/data/columnar/columnar_map.h"
#include "paimon/common/data/columnar/columnar_row_ref.h"
#include "paimon/common/utils/checked_cast.h"
#include "paimon/common/utils/date_time_utils.h"

namespace paimon {
Expand All @@ -44,8 +44,7 @@ Status ColumnarArray::CheckNoNull() const {

Decimal ColumnarArray::GetDecimal(int32_t pos, int32_t precision, int32_t scale) const {
using ArrayType = typename arrow::TypeTraits<arrow::Decimal128Type>::ArrayType;
auto array = arrow::internal::checked_cast<const ArrayType*>(array_);
assert(array);
auto array = checked_cast<const ArrayType*>(array_);
arrow::Decimal128 decimal(array->GetValue(offset_ + pos));
return Decimal(
precision, scale,
Expand All @@ -56,11 +55,9 @@ Decimal ColumnarArray::GetDecimal(int32_t pos, int32_t precision, int32_t scale)

Timestamp ColumnarArray::GetTimestamp(int32_t pos, int32_t precision) const {
using ArrayType = typename arrow::TypeTraits<arrow::TimestampType>::ArrayType;
auto array = arrow::internal::checked_cast<const ArrayType*>(array_);
assert(array);
auto array = checked_cast<const ArrayType*>(array_);
int64_t data = array->Value(offset_ + pos);
auto timestamp_type =
arrow::internal::checked_pointer_cast<arrow::TimestampType>(array->type());
auto timestamp_type = checked_pointer_cast<arrow::TimestampType>(array->type());
// for orc format, data is saved as nano, therefore, Timestamp convert should consider precision
// in arrow array rather than input precision
DateTimeUtils::TimeType time_type = DateTimeUtils::GetTimeTypeFromArrowType(timestamp_type);
Expand All @@ -70,25 +67,22 @@ Timestamp ColumnarArray::GetTimestamp(int32_t pos, int32_t precision) const {
}

std::shared_ptr<InternalArray> ColumnarArray::GetArray(int32_t pos) const {
auto list_array = arrow::internal::checked_cast<const arrow::ListArray*>(array_);
assert(list_array);
auto list_array = checked_cast<const arrow::ListArray*>(array_);
int32_t offset = list_array->value_offset(offset_ + pos);
int32_t length = list_array->value_length(offset_ + pos);
return std::make_shared<ColumnarArray>(list_array->values().get(), pool_, offset, length);
}

std::shared_ptr<InternalMap> ColumnarArray::GetMap(int32_t pos) const {
auto map_array = arrow::internal::checked_cast<const arrow::MapArray*>(array_);
assert(map_array);
auto map_array = checked_cast<const arrow::MapArray*>(array_);
int32_t offset = map_array->value_offset(offset_ + pos);
int32_t length = map_array->value_length(offset_ + pos);
return std::make_shared<ColumnarMap>(map_array->keys(), map_array->items(), pool_, offset,
length);
}

std::shared_ptr<InternalRow> ColumnarArray::GetRow(int32_t pos, int32_t num_fields) const {
auto struct_array = arrow::internal::checked_cast<const arrow::StructArray*>(array_);
assert(struct_array);
auto struct_array = checked_cast<const arrow::StructArray*>(array_);
auto row_ctx = std::make_shared<ColumnarBatchContext>(struct_array->fields(), pool_);
return std::make_shared<ColumnarRowRef>(std::move(row_ctx), offset_ + pos);
}
Expand Down
Loading
Loading