From 77725ce1a494337caec962019ab0f7f65178f435 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20=C3=81vila=20de=20Esp=C3=ADndola?= Date: Thu, 25 Jun 2020 10:16:39 -0700 Subject: [PATCH 1/4] big_decimal: Move constructors out of line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rafael Ávila de Espíndola --- utils/big_decimal.cc | 3 +++ utils/big_decimal.hh | 6 ++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/utils/big_decimal.cc b/utils/big_decimal.cc index 71162f32b0..665f6b508e 100644 --- a/utils/big_decimal.cc +++ b/utils/big_decimal.cc @@ -36,6 +36,9 @@ uint64_t from_varint_to_integer(const utils::multiprecision_int& varint) { return static_cast(~static_cast(0) & boost::multiprecision::cpp_int(varint)); } +big_decimal::big_decimal() : big_decimal(0, 0) {} +big_decimal::big_decimal(int32_t scale, boost::multiprecision::cpp_int unscaled_value) + : _scale(scale), _unscaled_value(std::move(unscaled_value)) {} big_decimal::big_decimal(sstring_view text) { diff --git a/utils/big_decimal.hh b/utils/big_decimal.hh index 34cb397a05..2cec2507ce 100644 --- a/utils/big_decimal.hh +++ b/utils/big_decimal.hh @@ -39,10 +39,8 @@ public: }; explicit big_decimal(sstring_view text); - big_decimal() : big_decimal(0, 0) {} - big_decimal(int32_t scale, boost::multiprecision::cpp_int unscaled_value) - : _scale(scale), _unscaled_value(unscaled_value) - { } + big_decimal(); + big_decimal(int32_t scale, boost::multiprecision::cpp_int unscaled_value); int32_t scale() const { return _scale; } const boost::multiprecision::cpp_int& unscaled_value() const { return _unscaled_value; } From bac0f3a9ee1d4f57d1ebb3695fc4951a0f3dac93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20=C3=81vila=20de=20Esp=C3=ADndola?= Date: Thu, 25 Jun 2020 10:36:41 -0700 Subject: [PATCH 2/4] big_decimal: Add a as_rational member function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This just refactors some duplicated code so that it can be fixed in one place. Signed-off-by: Rafael Ávila de Espíndola --- cql3/functions/castas_fcts.cc | 9 +++------ lua.cc | 10 ++++------ utils/big_decimal.cc | 8 ++++++++ utils/big_decimal.hh | 1 + 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/cql3/functions/castas_fcts.cc b/cql3/functions/castas_fcts.cc index 9076558a17..e5458d194b 100644 --- a/cql3/functions/castas_fcts.cc +++ b/cql3/functions/castas_fcts.cc @@ -88,16 +88,13 @@ static data_value castas_fctn_simple(data_value from) { template static data_value castas_fctn_from_decimal_to_float(data_value from) { auto val_from = value_cast(from); - boost::multiprecision::cpp_int ten(10); - boost::multiprecision::cpp_rational r = val_from.unscaled_value(); - r /= boost::multiprecision::pow(ten, val_from.scale()); - return static_cast(r); + return static_cast(val_from.as_rational()); } static utils::multiprecision_int from_decimal_to_cppint(const data_value& from) { const auto& val_from = value_cast(from); - boost::multiprecision::cpp_int ten(10); - return boost::multiprecision::cpp_int(val_from.unscaled_value() / boost::multiprecision::pow(ten, val_from.scale())); + auto r = val_from.as_rational(); + return utils::multiprecision_int(numerator(r)/denominator(r)); } template diff --git a/lua.cc b/lua.cc index 461dcc9469..9e873e066c 100644 --- a/lua.cc +++ b/lua.cc @@ -262,14 +262,12 @@ static auto visit_lua_raw_value(lua_State* l, int index, Func&& f) { template static auto visit_decimal(const big_decimal &v, Func&& f) { - boost::multiprecision::cpp_int ten(10); - const auto& dividend = v.unscaled_value(); - auto divisor = boost::multiprecision::pow(ten, v.scale()); + boost::multiprecision::cpp_rational r = v.as_rational(); + const boost::multiprecision::cpp_int& dividend = numerator(r); + const boost::multiprecision::cpp_int& divisor = denominator(r); if (dividend % divisor == 0) { - return f(utils::multiprecision_int(boost::multiprecision::cpp_int(dividend/divisor))); + return f(utils::multiprecision_int(dividend/divisor)); } - boost::multiprecision::cpp_rational r = dividend; - r /= divisor; return f(r.convert_to()); } diff --git a/utils/big_decimal.cc b/utils/big_decimal.cc index 665f6b508e..73d1ddfa57 100644 --- a/utils/big_decimal.cc +++ b/utils/big_decimal.cc @@ -85,6 +85,14 @@ big_decimal::big_decimal(sstring_view text) _scale += fraction.size(); } +boost::multiprecision::cpp_rational big_decimal::as_rational() const { + boost::multiprecision::cpp_int ten(10); + auto unscaled_value = static_cast(_unscaled_value); + boost::multiprecision::cpp_rational r = unscaled_value; + r /= boost::multiprecision::pow(ten, _scale); + return r; +} + sstring big_decimal::to_string() const { if (!_unscaled_value) { diff --git a/utils/big_decimal.hh b/utils/big_decimal.hh index 2cec2507ce..bcfb3e0022 100644 --- a/utils/big_decimal.hh +++ b/utils/big_decimal.hh @@ -44,6 +44,7 @@ public: int32_t scale() const { return _scale; } const boost::multiprecision::cpp_int& unscaled_value() const { return _unscaled_value; } + boost::multiprecision::cpp_rational as_rational() const; sstring to_string() const; From 684f32c8627d02f6d2582806fe6ebe269ae4106f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20=C3=81vila=20de=20Esp=C3=ADndola?= Date: Thu, 25 Jun 2020 10:25:18 -0700 Subject: [PATCH 3/4] big_decimal: Correctly handle negative scales MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A negative scale was being passed an a positive value to boost::multiprecision::pow, which would never finish. Signed-off-by: Rafael Ávila de Espíndola --- test/boost/castas_fcts_test.cc | 13 +++++++++++++ utils/big_decimal.cc | 8 +++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/test/boost/castas_fcts_test.cc b/test/boost/castas_fcts_test.cc index 812ad9f6b3..d5d2c6b025 100644 --- a/test/boost/castas_fcts_test.cc +++ b/test/boost/castas_fcts_test.cc @@ -142,6 +142,19 @@ SEASTAR_TEST_CASE(test_decimal_to_bigint) { }); } +SEASTAR_TEST_CASE(test_decimal_to_float) { + return do_with_cql_env_thread([&](auto& e) { + e.execute_cql("CREATE TABLE test (key text primary key, value decimal)").get(); + e.execute_cql("INSERT INTO test (key, value) VALUES ('k1', 10)").get(); + e.execute_cql("INSERT INTO test (key, value) VALUES ('k2', 1e1)").get(); + auto v = e.execute_cql("SELECT key, CAST(value as float) from test").get0(); + assert_that(v).is_rows().with_rows_ignore_order({ + {{serialized("k1")}, {serialized(float(10))}}, + {{serialized("k2")}, {serialized(float(10))}}, + }); + }); +} + SEASTAR_TEST_CASE(test_varint_to_bigint) { return do_with_cql_env_thread([&](auto& e) { e.execute_cql("CREATE TABLE test (key text primary key, value varint)").get(); diff --git a/utils/big_decimal.cc b/utils/big_decimal.cc index 73d1ddfa57..cdbf644761 100644 --- a/utils/big_decimal.cc +++ b/utils/big_decimal.cc @@ -89,7 +89,13 @@ boost::multiprecision::cpp_rational big_decimal::as_rational() const { boost::multiprecision::cpp_int ten(10); auto unscaled_value = static_cast(_unscaled_value); boost::multiprecision::cpp_rational r = unscaled_value; - r /= boost::multiprecision::pow(ten, _scale); + int32_t abs_scale = std::abs(_scale); + auto pow = boost::multiprecision::pow(ten, abs_scale); + if (_scale < 0) { + r *= pow; + } else { + r /= pow; + } return r; } From 85bb7ff743862472523405806848c133049dcee3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20=C3=81vila=20de=20Esp=C3=ADndola?= Date: Thu, 25 Jun 2020 14:35:38 -0700 Subject: [PATCH 4/4] big_decimal: Add a test for a corner case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This behavior is different from cassandra, but without arithmetic operations it doesn't seem possible to notice the difference from CQL. Using avg produces the same results, since we use an initial value of 0 (scale = 0). Signed-off-by: Rafael Ávila de Espíndola --- test/boost/big_decimal_test.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/boost/big_decimal_test.cc b/test/boost/big_decimal_test.cc index d5ad87eee7..c2b3b60f2c 100644 --- a/test/boost/big_decimal_test.cc +++ b/test/boost/big_decimal_test.cc @@ -157,6 +157,13 @@ BOOST_AUTO_TEST_CASE(test_big_decimal_div) { test_div("-0.25", 10, "-0.02"); test_div("-0.26", 10, "-0.03"); test_div("-10E10", 3, "-3E10"); + + // Document a small oddity, 1e1 has -1 decimal places, so dividing + // it by 2 produces 0. This is not the behavior in cassandra, but + // scylla doesn't expose arithmetic operations, so this doesn't + // seem to be visible from CQL. + test_div("10", 2, "5"); + test_div("1e1", 2, "0e1"); } BOOST_AUTO_TEST_CASE(test_big_decimal_assignadd) {