SteNicholas commented on code in PR #203:
URL: https://github.com/apache/paimon-cpp/pull/203#discussion_r3780766792
##########
src/paimon/core/mergetree/compact/aggregate/field_sum_agg.cpp:
##########
@@ -32,12 +32,16 @@ Result<FieldSumAgg::FieldSumFunc>
FieldSumAgg::CreateSumFunc(
arrow::Type::type type = field_type->id();
switch (type) {
case arrow::Type::type::INT8:
- return FieldSumFunc(
- [](const VariantType& accumulator, const VariantType&
input_field) -> VariantType {
- char sum = DataDefine::GetVariantValue<char>(accumulator) +
- DataDefine::GetVariantValue<char>(input_field);
- return sum;
- });
+ // The variant holds TINYINT as a plain char, whose signedness is
fixed by the
+ // target ABI. The mod-256 wrap of the stored byte is identical
either way, but add
+ // the signed values it stands for so the accumulator keeps Java's
byte semantics if
+ // it is ever widened or compared.
+ return FieldSumFunc([](const VariantType& accumulator,
+ const VariantType& input_field) ->
VariantType {
+ int8_t sum =
static_cast<int8_t>(DataDefine::GetVariantValue<char>(accumulator) +
Review Comment:
The current `clang-debug-x86_64` job is red because `check-clang-tidy`
reports `modernize-use-auto` on this declaration. Please use `const auto sum =
static_cast<int8_t>(...)` (or otherwise satisfy the check) so the required
Clang job passes:
https://github.com/apache/paimon-cpp/actions/runs/31763982737/job/94658447782
--
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]
