Skip to content

Commit

Permalink
Merge branch 'release-6.1' of github.com:pingcap/tiflash into cherry-…
Browse files Browse the repository at this point in the history
…pick-6796-to-release-6.1
  • Loading branch information
guo-shaoge committed Mar 29, 2023
2 parents a7cc635 + 7883d6f commit c93b49b
Show file tree
Hide file tree
Showing 10 changed files with 276 additions and 11 deletions.
2 changes: 1 addition & 1 deletion contrib/tiflash-proxy
Submodule tiflash-proxy updated 1 files
+2 −2 Cargo.lock
3 changes: 2 additions & 1 deletion dbms/src/Core/SortCursor.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#pragma once

#include <Columns/ColumnNullable.h>
#include <Columns/ColumnString.h>
#include <Columns/IColumn.h>
#include <Common/typeid_cast.h>
Expand Down Expand Up @@ -86,7 +87,7 @@ struct SortCursorImpl

sort_columns.push_back(block.safeGetByPosition(column_number).column.get());

need_collation[j] = desc[j].collator != nullptr && typeid_cast<const ColumnString *>(sort_columns.back()); /// TODO Nullable(String)
need_collation[j] = desc[j].collator != nullptr && typeid_cast<const ColumnString *>(std::get<0>(removeNullable(sort_columns.back())));
has_collation |= need_collation[j];
}

Expand Down
4 changes: 2 additions & 2 deletions dbms/src/Functions/FunctionsTiDBConversion.h
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ struct TiDBConvertToDecimal
const Context & context)
{
using UType = typename U::NativeType;
CastInternalType value = static_cast<CastInternalType>(v.value);
auto value = static_cast<CastInternalType>(v.value);

if (v_scale < scale)
{
Expand All @@ -953,8 +953,8 @@ struct TiDBConvertToDecimal
else if (v_scale > scale)
{
context.getDAGContext()->handleTruncateError("cast decimal as decimal");
value /= scale_mul;
const bool need_to_round = ((value < 0 ? -value : value) % scale_mul) >= (scale_mul / 2);
value /= scale_mul;
if (need_to_round)
{
if (value < 0)
Expand Down
37 changes: 32 additions & 5 deletions dbms/src/Functions/divide.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,36 @@ struct TiDBDivideFloatingImpl<A, B, false>
using ResultType = typename NumberTraits::ResultOfFloatingPointDivision<A, B>::Type;

template <typename Result = ResultType>
static Result apply(A a, B b)
static Result apply(A x, B d)
{
return static_cast<Result>(a) / b;
/// ref https://github.com/pingcap/tiflash/issues/6462
/// For division of Decimal/Decimal or Int/Decimal or Decimal/Int, we should round the result to make compatible with TiDB.
/// basically refer to https://stackoverflow.com/a/71634489
if constexpr (std::is_integral_v<Result> || std::is_same_v<Result, Int256> || std::is_same_v<Result, Int512>)
{
/// 1. do division first, get the quotient and mod, todo:(perf) find a unified `divmod` function to speed up this.
Result quotient = x / d;
Result mod = x % d;
/// 2. get the half of divisor, which is threshold to decide whether to round up or down.
/// note: don't directly use bit operation here, it may cause unexpected result.
Result half = (d / 2) + (d % 2);

/// 3. compare the abstract values of mod and half, if mod >= half, then round up.
Result abs_m = mod < 0 ? -mod : mod;
Result abs_h = half < 0 ? -half : half;
if (abs_m >= abs_h)
{
/// 4. now we need to round up, i.e., add 1 to the quotient's absolute value.
/// if the signs of dividend and divisor are the same, then the quotient should be positive, otherwise negative.
if ((x < 0) == (d < 0)) // same_sign, i.e., quotient >= 0
quotient = quotient + 1;
else
quotient = quotient - 1;
}
return quotient;
}
else
return static_cast<Result>(x) / d;
}
template <typename Result = ResultType>
static Result apply(A a, B b, UInt8 & res_null)
Expand All @@ -75,7 +102,7 @@ struct TiDBDivideFloatingImpl<A, B, false>
res_null = 1;
return static_cast<Result>(0);
}
return static_cast<Result>(a) / b;
return apply<Result>(a, b);
}
};

Expand All @@ -102,7 +129,7 @@ struct TiDBDivideFloatingImpl<A, B, true>
res_null = 1;
return static_cast<Result>(0);
}
return static_cast<Result>(a) / static_cast<Result>(b);
return apply<Result>(a, b);
}
};

Expand Down Expand Up @@ -332,4 +359,4 @@ void registerFunctionDivideIntegralOrZero(FunctionFactory & factory)
factory.registerFunction<FunctionDivideIntegralOrZero>();
}

} // namespace DB
} // namespace DB
138 changes: 138 additions & 0 deletions dbms/src/Functions/tests/gtest_arithmetic_functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
#include <Interpreters/Context.h>
#include <TestUtils/FunctionTestUtils.h>
#include <TestUtils/TiFlashTestBasic.h>
#include <gtest/gtest.h>

#include <Functions/divide.cpp>
#include <string>
#include <unordered_map>
#include <vector>
Expand Down Expand Up @@ -103,6 +105,142 @@ class TestBinaryArithmeticFunctions : public DB::tests::FunctionTest
}
};

template <typename TYPE>
void doTiDBDivideDecimalRoundInternalTest()
{
auto apply = static_cast<TYPE (*)(TYPE, TYPE)>(&TiDBDivideFloatingImpl<TYPE, TYPE, false>::apply);

constexpr TYPE max = std::numeric_limits<TYPE>::max();
// note: Int256's min is not equal to -max-1
// according to https://www.boost.org/doc/libs/1_60_0/libs/multiprecision/doc/html/boost_multiprecision/tut/ints/cpp_int.html
constexpr TYPE min = std::numeric_limits<TYPE>::min();

// clang-format off
const std::vector<std::array<TYPE, 3>> cases = {
{1, 2, 1}, {1, -2, -1}, {-1, 2, -1}, {-1, -2, 1},

{0, 3, 0}, {0, -3, 0}, {0, 3, 0}, {0, -3, 0},
{1, 3, 0}, {1, -3, 0}, {-1, 3, 0}, {-1, -3, 0},
{2, 3, 1}, {2, -3, -1}, {-2, 3, -1}, {-2, -3, 1},
{3, 3, 1}, {3, -3, -1}, {-3, 3, -1}, {-3, -3, 1},
{4, 3, 1}, {4, -3, -1}, {-4, 3, -1}, {-4, -3, 1},
{5, 3, 2}, {5, -3, -2}, {-5, 3, -2}, {-5, -3, 2},

// ±max as divisor
{0, max, 0}, {max/2-1, max, 0}, {max/2, max, 0}, {max/2+1, max, 1}, {max-1, max, 1}, {max, max, 1},
{-1, max, 0}, {-max/2+1, max, 0}, {-max/2, max, 0}, {-max/2-1, max, -1}, {-max+1, max, -1}, {-max, max, -1}, {min, max, -1},
{0, -max, 0}, {max/2-1, -max, 0}, {max/2, -max, 0}, {max/2+1, -max, -1}, {max-1, -max, -1}, {max, -max, -1},
{-1, -max, 0}, {-max/2+1, -max, 0}, {-max/2, -max, 0}, {-max/2-1, -max, 1}, {-max+1, -max, 1}, {-max, -max, 1}, {min, -max, 1},

// ±max as dividend
{max, 1, max}, {max, 2, max/2+1}, {max, max/2-1, 2}, {max, max/2, 2}, {max, max/2+1, 2}, {max, max-1, 1},
{max, -1, -max}, {max, -2, -max/2-1}, {max, -max/2+1, -2}, {max, -max/2, -2}, {max, -max/2-1, -2}, {max, -max+1, -1},
{-max, 1, -max}, {-max, 2, -max/2-1}, {-max, max/2+1, -2}, {-max, max/2, -2}, {-max, max/2-1, -2}, {-max, max-1, -1},
{-max, -1, max}, {-max, -2, max/2+1}, {-max, -max/2-1, 2}, {-max, -max/2, 2}, {-max, -max/2+1, 2}, {-max, -max+1, 1},
};
// clang-format on

for (const auto & expect : cases)
{
std::array<TYPE, 3> actual = {expect[0], expect[1], apply(expect[0], expect[1])};
ASSERT_EQ(expect, actual);
}
}

TEST_F(TestBinaryArithmeticFunctions, TiDBDivideDecimalRoundInternal)
try
{
doTiDBDivideDecimalRoundInternalTest<Int32>();
doTiDBDivideDecimalRoundInternalTest<Int64>();
doTiDBDivideDecimalRoundInternalTest<Int128>();
doTiDBDivideDecimalRoundInternalTest<Int256>();
doTiDBDivideDecimalRoundInternalTest<Int512>();
}
CATCH

TEST_F(TestBinaryArithmeticFunctions, TiDBDivideDecimalRound)
try
{
const String func_name = "tidbDivide";

// decimal32
{
// int and decimal
ASSERT_COLUMN_EQ(
createColumn<Nullable<Decimal64>>(std::make_tuple(18, 4), {DecimalField64(1, 4), DecimalField64(1, 4), DecimalField64(1, 4), DecimalField64(1, 4), DecimalField64(0, 4)}),
executeFunction(
func_name,
createColumn<Int32>({1, 1, 1, 1, 1}),
createColumn<Decimal32>(std::make_tuple(20, 4), {DecimalField32(100000000, 4), DecimalField32(100010000, 4), DecimalField32(199990000, 4), DecimalField32(200000000, 4), DecimalField32(200010000, 4)})));

// decimal and decimal
ASSERT_COLUMN_EQ(
createColumn<Nullable<Decimal128>>(std::make_tuple(26, 8), {DecimalField128(10000, 8), DecimalField128(9999, 8), DecimalField128(5000, 8), DecimalField128(5000, 8), DecimalField128(5000, 8)}),
executeFunction(
func_name,
createColumn<Decimal32>(std::make_tuple(18, 4), {DecimalField32(10000, 4), DecimalField32(10000, 4), DecimalField32(10000, 4), DecimalField32(10000, 4), DecimalField32(10000, 4)}),
createColumn<Decimal32>(std::make_tuple(18, 4), {DecimalField32(100000000, 4), DecimalField32(100010000, 4), DecimalField32(199990000, 4), DecimalField32(200000000, 4), DecimalField32(200010000, 4)})));
}

// decimal64
{
// int and decimal
ASSERT_COLUMN_EQ(
createColumn<Nullable<Decimal64>>(std::make_tuple(18, 4), {DecimalField64(1, 4), DecimalField64(1, 4), DecimalField64(1, 4), DecimalField64(1, 4), DecimalField64(0, 4)}),
executeFunction(
func_name,
createColumn<Int32>({1, 1, 1, 1, 1}),
createColumn<Decimal64>(std::make_tuple(20, 4), {DecimalField64(100000000, 4), DecimalField64(100010000, 4), DecimalField64(199990000, 4), DecimalField64(200000000, 4), DecimalField64(200010000, 4)})));

// decimal and decimal
ASSERT_COLUMN_EQ(
createColumn<Nullable<Decimal128>>(std::make_tuple(26, 8), {DecimalField128(10000, 8), DecimalField128(9999, 8), DecimalField128(5000, 8), DecimalField128(5000, 8), DecimalField128(5000, 8)}),
executeFunction(
func_name,
createColumn<Decimal64>(std::make_tuple(18, 4), {DecimalField64(10000, 4), DecimalField64(10000, 4), DecimalField64(10000, 4), DecimalField64(10000, 4), DecimalField64(10000, 4)}),
createColumn<Decimal64>(std::make_tuple(18, 4), {DecimalField64(100000000, 4), DecimalField64(100010000, 4), DecimalField64(199990000, 4), DecimalField64(200000000, 4), DecimalField64(200010000, 4)})));
}

// decimal128
{
// int and decimal
ASSERT_COLUMN_EQ(
createColumn<Nullable<Decimal64>>(std::make_tuple(18, 4), {DecimalField64(1, 4), DecimalField64(1, 4), DecimalField64(1, 4), DecimalField64(1, 4), DecimalField64(0, 4)}),
executeFunction(
func_name,
createColumn<Int32>({1, 1, 1, 1, 1}),
createColumn<Decimal128>(std::make_tuple(20, 4), {DecimalField128(100000000, 4), DecimalField128(100010000, 4), DecimalField128(199990000, 4), DecimalField128(200000000, 4), DecimalField128(200010000, 4)})));

// decimal and decimal
ASSERT_COLUMN_EQ(
createColumn<Nullable<Decimal128>>(std::make_tuple(26, 8), {DecimalField128(10000, 8), DecimalField128(9999, 8), DecimalField128(5000, 8), DecimalField128(5000, 8), DecimalField128(5000, 8)}),
executeFunction(
func_name,
createColumn<Decimal128>(std::make_tuple(18, 4), {DecimalField128(10000, 4), DecimalField128(10000, 4), DecimalField128(10000, 4), DecimalField128(10000, 4), DecimalField128(10000, 4)}),
createColumn<Decimal128>(std::make_tuple(18, 4), {DecimalField128(100000000, 4), DecimalField128(100010000, 4), DecimalField128(199990000, 4), DecimalField128(200000000, 4), DecimalField128(200010000, 4)})));
}

// decimal256
{
// int and decimal
ASSERT_COLUMN_EQ(
createColumn<Nullable<Decimal64>>(std::make_tuple(18, 4), {DecimalField64(1, 4), DecimalField64(1, 4), DecimalField64(1, 4), DecimalField64(1, 4), DecimalField64(0, 4)}),
executeFunction(
func_name,
createColumn<Int32>({1, 1, 1, 1, 1}),
createColumn<Decimal256>(std::make_tuple(20, 4), {DecimalField256(Int256(100000000), 4), DecimalField256(Int256(100010000), 4), DecimalField256(Int256(199990000), 4), DecimalField256(Int256(200000000), 4), DecimalField256(Int256(200010000), 4)})));

// decimal and decimal
ASSERT_COLUMN_EQ(
createColumn<Nullable<Decimal128>>(std::make_tuple(26, 8), {DecimalField128(10000, 8), DecimalField128(9999, 8), DecimalField128(5000, 8), DecimalField128(5000, 8), DecimalField128(5000, 8)}),
executeFunction(
func_name,
createColumn<Decimal256>(std::make_tuple(18, 4), {DecimalField256(Int256(10000), 4), DecimalField256(Int256(10000), 4), DecimalField256(Int256(10000), 4), DecimalField256(Int256(10000), 4), DecimalField256(Int256(10000), 4)}),
createColumn<Decimal256>(std::make_tuple(18, 4), {DecimalField256(Int256(100000000), 4), DecimalField256(Int256(100010000), 4), DecimalField256(Int256(199990000), 4), DecimalField256(Int256(200000000), 4), DecimalField256(Int256(200010000), 4)})));
}
}
CATCH

TEST_F(TestBinaryArithmeticFunctions, TiDBDivideDecimal)
try
{
Expand Down
55 changes: 54 additions & 1 deletion dbms/src/Functions/tests/gtest_tidb_conversion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1247,6 +1247,59 @@ try
}
CATCH

TEST_F(TestTidbConversion, castDecimalAsDecimalWithRound)
try
{
DAGContext * dag_context = context.getDAGContext();
UInt64 ori_flags = dag_context->getFlags();
dag_context->addFlag(TiDBSQLFlags::TRUNCATE_AS_WARNING);
dag_context->clearWarnings();

/// decimal32 to decimal32/64/128/256
ASSERT_COLUMN_EQ(createColumn<Decimal32>(std::make_tuple(5, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal32>(std::make_tuple(5, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(5,2)")}));
ASSERT_COLUMN_EQ(createColumn<Decimal64>(std::make_tuple(15, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal32>(std::make_tuple(5, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(15,2)")}));
ASSERT_COLUMN_EQ(createColumn<Decimal128>(std::make_tuple(25, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal32>(std::make_tuple(5, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(25,2)")}));
ASSERT_COLUMN_EQ(createColumn<Decimal256>(std::make_tuple(45, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal32>(std::make_tuple(5, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(45,2)")}));

/// decimal64 to decimal32/64/128/256
ASSERT_COLUMN_EQ(createColumn<Decimal32>(std::make_tuple(5, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal64>(std::make_tuple(15, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(5,2)")}));
ASSERT_COLUMN_EQ(createColumn<Decimal64>(std::make_tuple(15, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal64>(std::make_tuple(15, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(15,2)")}));
ASSERT_COLUMN_EQ(createColumn<Decimal128>(std::make_tuple(25, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal64>(std::make_tuple(15, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(25,2)")}));
ASSERT_COLUMN_EQ(createColumn<Decimal256>(std::make_tuple(45, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal64>(std::make_tuple(15, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(45,2)")}));

/// decimal128 to decimal32/64/128/256
ASSERT_COLUMN_EQ(createColumn<Decimal32>(std::make_tuple(5, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal128>(std::make_tuple(25, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(5,2)")}));
ASSERT_COLUMN_EQ(createColumn<Decimal64>(std::make_tuple(15, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal128>(std::make_tuple(25, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(15,2)")}));
ASSERT_COLUMN_EQ(createColumn<Decimal128>(std::make_tuple(25, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal128>(std::make_tuple(25, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(25,2)")}));
ASSERT_COLUMN_EQ(createColumn<Decimal256>(std::make_tuple(45, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal128>(std::make_tuple(25, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(45,2)")}));

/// decimal256 to decimal32/64/128/256
ASSERT_COLUMN_EQ(createColumn<Decimal32>(std::make_tuple(5, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal256>(std::make_tuple(45, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(5,2)")}));
ASSERT_COLUMN_EQ(createColumn<Decimal64>(std::make_tuple(15, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal256>(std::make_tuple(45, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(15,2)")}));
ASSERT_COLUMN_EQ(createColumn<Decimal128>(std::make_tuple(25, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal256>(std::make_tuple(45, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(25,2)")}));
ASSERT_COLUMN_EQ(createColumn<Decimal256>(std::make_tuple(45, 2), {"1.23", "1.56", "1.01", "1.00", "-1.23", "-1.56", "-1.01", "-1.00"}),
executeFunction(func_name, {createColumn<Decimal256>(std::make_tuple(45, 4), {"1.2300", "1.5600", "1.0056", "1.0023", "-1.2300", "-1.5600", "-1.0056", "-1.0023"}), createCastTypeConstColumn("Decimal(45,2)")}));

dag_context->setFlags(ori_flags);
dag_context->clearWarnings();
}
CATCH

TEST_F(TestTidbConversion, castTimeAsReal)
try
{
Expand Down Expand Up @@ -1563,7 +1616,7 @@ TEST_F(TestTidbConversion, skipCheckOverflowIntToDeciaml)
ASSERT_TRUE(FunctionTiDBCast::canSkipCheckOverflowForDecimal<DataTypeUInt64>(uint64_ptr, prec_decimal256, scale));
}

TEST_F(TestTidbConversion, skipCheckOverflowDecimalToDeciaml)
TEST_F(TestTidbConversion, skipCheckOverflowDecimalToDecimal)
{
DataTypePtr decimal32_ptr_8_3 = createDecimal(8, 3);
DataTypePtr decimal32_ptr_8_2 = createDecimal(8, 2);
Expand Down
File renamed without changes.
File renamed without changes.
Loading

0 comments on commit c93b49b

Please sign in to comment.