Copilot commented on code in PR #54557:
URL: https://github.com/apache/doris/pull/54557#discussion_r2268152827
##########
be/src/vec/sink/writer/iceberg/partition_transformers.h:
##########
@@ -343,11 +343,10 @@ class DecimalTruncatePartitionColumnTransform : public
PartitionColumnTransform
auto col_res = ColumnDecimal<PT>::create(vec_src.size(),
decimal_col->get_scale());
auto& vec_res = col_res->get_data();
- const typename T::NativeType* __restrict p_in =
- reinterpret_cast<const T::NativeType*>(vec_src.data());
- const typename T::NativeType* end_in =
+ const auto* __restrict p_in = reinterpret_cast<const
T::NativeType*>(vec_src.data());
Review Comment:
The change from `typename T::NativeType*` to `auto*` removes important type
information. This could mask type safety issues in template code where
`T::NativeType` might not match the expected type from `vec_src.data()`.
```suggestion
const typename T::NativeType* __restrict p_in =
reinterpret_cast<const T::NativeType*>(vec_src.data());
```
##########
be/src/vec/sink/writer/iceberg/partition_transformers.h:
##########
@@ -343,11 +343,10 @@ class DecimalTruncatePartitionColumnTransform : public
PartitionColumnTransform
auto col_res = ColumnDecimal<PT>::create(vec_src.size(),
decimal_col->get_scale());
auto& vec_res = col_res->get_data();
- const typename T::NativeType* __restrict p_in =
- reinterpret_cast<const T::NativeType*>(vec_src.data());
- const typename T::NativeType* end_in =
+ const auto* __restrict p_in = reinterpret_cast<const
T::NativeType*>(vec_src.data());
+ const auto* end_in =
reinterpret_cast<const T::NativeType*>(vec_src.data()) +
vec_src.size();
- typename T::NativeType* __restrict p_out =
reinterpret_cast<T::NativeType*>(vec_res.data());
+ auto* __restrict p_out =
reinterpret_cast<T::NativeType*>(vec_res.data());
Review Comment:
The change from `typename T::NativeType*` to `auto*` removes explicit type
specification. Since this is a `reinterpret_cast` to `T::NativeType*`, the
explicit type should be maintained for clarity and type safety.
```suggestion
typename T::NativeType* __restrict p_out =
reinterpret_cast<T::NativeType*>(vec_res.data());
```
##########
be/src/vec/sink/writer/iceberg/partition_transformers.h:
##########
@@ -527,13 +526,11 @@ class DecimalBucketPartitionColumnTransform : public
PartitionColumnTransform {
auto col_res = ColumnInt32::create();
ColumnInt32::Container& out_data = col_res->get_data();
out_data.resize(in_data.size());
- auto& vec_res = col_res->get_data();
- const typename T::NativeType* __restrict p_in =
- reinterpret_cast<const T::NativeType*>(in_data.data());
- const typename T::NativeType* end_in =
+ const auto* __restrict p_in = reinterpret_cast<const
T::NativeType*>(in_data.data());
+ const auto* end_in =
Review Comment:
Similar to line 346, changing from `typename T::NativeType*` to `auto*`
reduces type safety in template code where explicit type specification is
important for maintainability and correctness.
```suggestion
const T::NativeType* __restrict p_in = reinterpret_cast<const
T::NativeType*>(in_data.data());
const T::NativeType* end_in =
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
