Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: format throw data truncated error #5272

Merged
merged 21 commits into from
Jul 11, 2022
Merged

fix: format throw data truncated error #5272

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
c99bc07
fix
xzhangxian1008 Jul 1, 2022
d21ac2f
update
xzhangxian1008 Jul 1, 2022
be0699b
Merge branch 'master' into issue_4891
xzhangxian1008 Jul 4, 2022
ba3b1c1
Merge branch 'master' into issue_4891
xzhangxian1008 Jul 4, 2022
2e49d56
Merge branch 'master' into issue_4891
xzhangxian1008 Jul 5, 2022
726e12f
update
xzhangxian1008 Jul 5, 2022
40f1a4d
format
xzhangxian1008 Jul 5, 2022
8d6c677
Merge branch 'master' into issue_4891
xzhangxian1008 Jul 5, 2022
339014a
update
xzhangxian1008 Jul 5, 2022
e931521
format
xzhangxian1008 Jul 5, 2022
3f5e6df
Merge branch 'master' of https://github.com/pingcap/tiflash into issu…
xzhangxian1008 Jul 7, 2022
58e7a98
Merge branch 'master' into issue_4891
xzhangxian1008 Jul 8, 2022
01dd580
enable ut
xzhangxian1008 Jul 8, 2022
107e110
Merge branch 'master' into issue_4891
xzhangxian1008 Jul 8, 2022
ca01708
Merge branch 'master' into issue_4891
xzhangxian1008 Jul 9, 2022
dba518e
update
xzhangxian1008 Jul 9, 2022
50f4598
update
xzhangxian1008 Jul 9, 2022
d6e8f15
udpate
xzhangxian1008 Jul 11, 2022
ee26f15
Merge branch 'master' into issue_4891
xzhangxian1008 Jul 11, 2022
aeab46a
add fullstack test
xzhangxian1008 Jul 11, 2022
ceac0fd
Merge branch 'master' into issue_4891
xzhangxian1008 Jul 11, 2022
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
13 changes: 10 additions & 3 deletions dbms/src/Functions/FunctionsRound.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1126,6 +1126,16 @@ struct TiDBDecimalRoundInfo
, output_prec(getDecimalPrecision(output_type, 0))
, output_scale(getDecimalScale(output_type, 0))
{}

TiDBDecimalRoundInfo(const FracType & input_prec_, const FracType & input_scale_, const FracType & output_prec_, const FracType & output_scale_)
: input_prec(input_prec_)
, input_scale(input_scale_)
, input_int_prec(input_prec - input_scale)
, output_prec(output_prec_)
, output_scale(output_scale_)
{}

void setOutputScale(const FracType & output_scale_) { output_scale = output_scale_; }
};

template <typename InputType, typename OutputType>
Expand Down Expand Up @@ -1182,9 +1192,6 @@ struct TiDBDecimalRound

auto scaled_value = static_cast<UnsignedOutput>(absolute_value);

if (difference < 0)
scaled_value *= PowForOutput::result[-difference];

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove this?

// check overflow and construct result.
if (scaled_value > DecimalMaxValue::get(info.output_prec))
throw TiFlashException("Data truncated", Errors::Decimal::Overflow);
Expand Down
15 changes: 13 additions & 2 deletions dbms/src/Functions/FunctionsString.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4565,6 +4565,9 @@ class FormatImpl : public IFunction
const auto & number_base_type = block.getByPosition(arguments[0]).type;
const auto & precision_base_type = block.getByPosition(arguments[1]).type;

auto precision_base_col = block.getByPosition(arguments[1]).column;
auto precision_base_col_size = precision_base_col->size();

auto col_res = ColumnString::create();
auto val_num = block.getByPosition(arguments[0]).column->size();

Expand All @@ -4573,7 +4576,14 @@ class FormatImpl : public IFunction
using NumberFieldType = typename NumberType::FieldType;
using NumberColVec = std::conditional_t<IsDecimal<NumberFieldType>, ColumnDecimal<NumberFieldType>, ColumnVector<NumberFieldType>>;
const auto * number_raw = block.getByPosition(arguments[0]).column.get();
TiDBDecimalRoundInfo info{number_type, number_type};

auto prec = getDecimalPrecision(number_type, 0);
auto scale = getDecimalScale(number_type, 0);

/// output_scale is the second parameter of the 'format' function
auto output_scale = precision_base_col_size > 0 ? (*precision_base_col)[0].get<Int64>() : scale;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if precision_base_col is not a constant column, in this case the output_scale is differ from row to row.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if precision_base_col is not a constant column, in this case the output_scale is differ from row to row.

When precision_base_col is not a constant column, we will always reset the info.output_scale with max_num_decimals which can be shown in FunctionsString.cpp:4625.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then you need to update info.output_scale in Line 4603.

output_scale = output_scale < 0 ? 0 : output_scale; /// Ensure output_scale >= 0
TiDBDecimalRoundInfo info{prec, scale, prec, output_scale};

return getPrecisionType(precision_base_type, [&](const auto & precision_type, bool) {
using PrecisionType = std::decay_t<decltype(precision_type)>;
Expand Down Expand Up @@ -4612,6 +4622,7 @@ class FormatImpl : public IFunction
for (size_t i = 0; i != val_num; ++i)
{
size_t max_num_decimals = getMaxNumDecimals(precision_array[i]);
info.setOutputScale(max_num_decimals);
format(number_array[i], max_num_decimals, info, col_res->getChars(), col_res->getOffsets());
}
}
Expand Down Expand Up @@ -4700,7 +4711,7 @@ class FormatImpl : public IFunction
static std::string number2Str(T number, const TiDBDecimalRoundInfo & info [[maybe_unused]])
{
if constexpr (IsDecimal<T>)
return number.toString(info.output_scale);
return number.toString(std::min(info.input_scale, info.output_scale));
else
{
static_assert(std::is_floating_point_v<T> || std::is_integral_v<T>);
Expand Down
9 changes: 6 additions & 3 deletions dbms/src/Functions/tests/gtest_strings_format.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class StringFormat : public DB::tests::FunctionTest
using FieldType = DecimalField<Decimal>;
using NullableDecimal = Nullable<Decimal>;
ASSERT_COLUMN_EQ(
createColumn<Nullable<String>>({"0.0000", "-0.0120", "0.0120", "12,332.1000", "12,332", "12,332", "12,332.300000000000000000000000000000", "-12,332.30000", "-1,000.0", "-333.33", {}}),
createColumn<Nullable<String>>({"0.0000", "-0.0120", "0.0120", "12,332.1000", "12,332", "12,332", "12,332.300000000000000000000000000000", "-12,332.30000", "-1,000.0", "-333.33", {}, "99,999.9999000000", "100,000.000", "100,000"}),
executeFunction(
func_name,
createColumn<NullableDecimal>(
Expand All @@ -49,8 +49,11 @@ class StringFormat : public DB::tests::FunctionTest
FieldType(static_cast<Native>(-123323000), 4),
FieldType(static_cast<Native>(-9999999), 4),
FieldType(static_cast<Native>(-3333330), 4),
FieldType(static_cast<Native>(0), 0)}),
createColumn<Nullable<Int64>>({4, 4, 4, 4, 0, -1, 31, 5, 1, 2, {}})));
FieldType(static_cast<Native>(0), 0),
FieldType(static_cast<Native>(999999999), 4),
FieldType(static_cast<Native>(999999999), 4),
FieldType(static_cast<Native>(999999999), 4)}),
createColumn<Nullable<Int64>>({4, 4, 4, 4, 0, -1, 31, 5, 1, 2, {}, 10, 3, -5})));
ASSERT_COLUMN_EQ(
createColumn<Nullable<String>>({"12,332.100", "-12,332.300", "-1,000.000", "-333.333"}),
executeFunction(
Expand Down