From 15ef5b685f8e01c01cb76520d275aa218ab2b4ce Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Mon, 27 Dec 2021 17:26:52 +0800 Subject: [PATCH] round the float/double when insert it to integer column --- src/codec/RowWriterV2.cpp | 18 ++++--- src/graph/executor/admin/SpaceExecutor.cpp | 2 +- src/graph/service/GraphService.cpp | 6 ++- src/graph/service/QueryInstance.cpp | 10 ++-- tests/tck/features/bugfix/RoundFloat.feature | 55 ++++++++++++++++++++ 5 files changed, 76 insertions(+), 15 deletions(-) create mode 100644 tests/tck/features/bugfix/RoundFloat.feature diff --git a/src/codec/RowWriterV2.cpp b/src/codec/RowWriterV2.cpp index d1c4cbf0613..639b4854965 100644 --- a/src/codec/RowWriterV2.cpp +++ b/src/codec/RowWriterV2.cpp @@ -5,6 +5,8 @@ #include "codec/RowWriterV2.h" +#include + #include "common/time/TimeUtils.h" #include "common/time/WallClock.h" #include "common/utils/DefaultValueContext.h" @@ -286,7 +288,7 @@ WriteResult RowWriterV2::write(ssize_t index, float v) noexcept { if (v > std::numeric_limits::max() || v < std::numeric_limits::min()) { return WriteResult::OUT_OF_RANGE; } - int8_t iv = v; + int8_t iv = std::round(v); buf_[offset] = iv; break; } @@ -294,7 +296,7 @@ WriteResult RowWriterV2::write(ssize_t index, float v) noexcept { if (v > std::numeric_limits::max() || v < std::numeric_limits::min()) { return WriteResult::OUT_OF_RANGE; } - int16_t iv = v; + int16_t iv = std::round(v); memcpy(&buf_[offset], reinterpret_cast(&iv), sizeof(int16_t)); break; } @@ -303,7 +305,7 @@ WriteResult RowWriterV2::write(ssize_t index, float v) noexcept { v < static_cast(std::numeric_limits::min())) { return WriteResult::OUT_OF_RANGE; } - int32_t iv = v; + int32_t iv = std::round(v); memcpy(&buf_[offset], reinterpret_cast(&iv), sizeof(int32_t)); break; } @@ -312,7 +314,7 @@ WriteResult RowWriterV2::write(ssize_t index, float v) noexcept { v < static_cast(std::numeric_limits::min())) { return WriteResult::OUT_OF_RANGE; } - int64_t iv = v; + int64_t iv = std::round(v); memcpy(&buf_[offset], reinterpret_cast(&iv), sizeof(int64_t)); break; } @@ -343,7 +345,7 @@ WriteResult RowWriterV2::write(ssize_t index, double v) noexcept { if (v > std::numeric_limits::max() || v < std::numeric_limits::min()) { return WriteResult::OUT_OF_RANGE; } - int8_t iv = v; + int8_t iv = std::round(v); buf_[offset] = iv; break; } @@ -351,7 +353,7 @@ WriteResult RowWriterV2::write(ssize_t index, double v) noexcept { if (v > std::numeric_limits::max() || v < std::numeric_limits::min()) { return WriteResult::OUT_OF_RANGE; } - int16_t iv = v; + int16_t iv = std::round(v); memcpy(&buf_[offset], reinterpret_cast(&iv), sizeof(int16_t)); break; } @@ -359,7 +361,7 @@ WriteResult RowWriterV2::write(ssize_t index, double v) noexcept { if (v > std::numeric_limits::max() || v < std::numeric_limits::min()) { return WriteResult::OUT_OF_RANGE; } - int32_t iv = v; + int32_t iv = std::round(v); memcpy(&buf_[offset], reinterpret_cast(&iv), sizeof(int32_t)); break; } @@ -368,7 +370,7 @@ WriteResult RowWriterV2::write(ssize_t index, double v) noexcept { v < static_cast(std::numeric_limits::min())) { return WriteResult::OUT_OF_RANGE; } - int64_t iv = v; + int64_t iv = std::round(v); memcpy(&buf_[offset], reinterpret_cast(&iv), sizeof(int64_t)); break; } diff --git a/src/graph/executor/admin/SpaceExecutor.cpp b/src/graph/executor/admin/SpaceExecutor.cpp index 9591b734b32..e7f9cc52a62 100644 --- a/src/graph/executor/admin/SpaceExecutor.cpp +++ b/src/graph/executor/admin/SpaceExecutor.cpp @@ -167,7 +167,7 @@ folly::Future DropSpaceExecutor::execute() { } void DropSpaceExecutor::unRegisterSpaceLevelMetrics(const std::string &spaceName) { - if (FLAGS_enable_space_level_metrics) { + if (FLAGS_enable_space_level_metrics && spaceName != "") { stats::StatsManager::removeCounterWithLabels(kNumQueries, {{"space", spaceName}}); stats::StatsManager::removeCounterWithLabels(kNumSlowQueries, {{"space", spaceName}}); stats::StatsManager::removeCounterWithLabels(kNumQueryErrors, {{"space", spaceName}}); diff --git a/src/graph/service/GraphService.cpp b/src/graph/service/GraphService.cpp index 0417399c1fc..73f903526e6 100644 --- a/src/graph/service/GraphService.cpp +++ b/src/graph/service/GraphService.cpp @@ -161,8 +161,10 @@ folly::Future GraphService::future_executeWithParameter( return ctx->finish(); } stats::StatsManager::addValue(kNumQueries); - stats::StatsManager::addValue( - stats::StatsManager::counterWithLabels(kNumQueries, {{"space", sessionPtr->space().name}})); + if (FLAGS_enable_space_level_metrics && sessionPtr->space().name != "") { + stats::StatsManager::addValue(stats::StatsManager::counterWithLabels( + kNumQueries, {{"space", sessionPtr->space().name}})); + } ctx->setSession(std::move(sessionPtr)); ctx->setParameterMap(parameterMap); queryEngine_->execute(std::move(ctx)); diff --git a/src/graph/service/QueryInstance.cpp b/src/graph/service/QueryInstance.cpp index 4b42a5c9a7f..09d6769f90d 100644 --- a/src/graph/service/QueryInstance.cpp +++ b/src/graph/service/QueryInstance.cpp @@ -163,8 +163,10 @@ void QueryInstance::onError(Status status) { auto latency = rctx->duration().elapsedInUSec(); rctx->resp().latencyInUs = latency; stats::StatsManager::addValue(kNumQueryErrors); - stats::StatsManager::addValue( - stats::StatsManager::counterWithLabels(kNumQueryErrors, {{"space", spaceName}})); + if (FLAGS_enable_space_level_metrics && spaceName != "") { + stats::StatsManager::addValue( + stats::StatsManager::counterWithLabels(kNumQueryErrors, {{"space", spaceName}})); + } addSlowQueryStats(latency, spaceName); rctx->session()->deleteQuery(qctx_.get()); rctx->finish(); @@ -173,14 +175,14 @@ void QueryInstance::onError(Status status) { void QueryInstance::addSlowQueryStats(uint64_t latency, const std::string &spaceName) const { stats::StatsManager::addValue(kQueryLatencyUs, latency); - if (FLAGS_enable_space_level_metrics) { + if (FLAGS_enable_space_level_metrics && spaceName != "") { stats::StatsManager::addValue( stats::StatsManager::histoWithLabels(kQueryLatencyUs, {{"space", spaceName}}), latency); } if (latency > static_cast(FLAGS_slow_query_threshold_us)) { stats::StatsManager::addValue(kNumSlowQueries); stats::StatsManager::addValue(kSlowQueryLatencyUs, latency); - if (FLAGS_enable_space_level_metrics) { + if (FLAGS_enable_space_level_metrics && spaceName != "") { stats::StatsManager::addValue( stats::StatsManager::counterWithLabels(kNumSlowQueries, {{"space", spaceName}})); stats::StatsManager::addValue( diff --git a/tests/tck/features/bugfix/RoundFloat.feature b/tests/tck/features/bugfix/RoundFloat.feature new file mode 100644 index 00000000000..307e42a8ac3 --- /dev/null +++ b/tests/tck/features/bugfix/RoundFloat.feature @@ -0,0 +1,55 @@ +# Copyright (c) 2021 vesoft inc. All rights reserved. +# +# This source code is licensed under Apache 2.0 License. +Feature: Round the float/double when insert them into integer column + + # issue https://github.com/vesoft-inc/nebula/issues/3473 + Scenario: Insert float/double into a integer column + Given an empty graph + And create a space with following options: + | partition_num | 9 | + | replica_factor | 1 | + | vid_type | FIXED_STRING(30) | + | charset | utf8 | + | collate | utf8_bin | + When executing query: + """ + create tag test(a int32); + """ + Then the execution should be successful + When try to execute query: + """ + INSERT VERTEX test(a) VALUES '101':(3.2); + """ + Then the execution should be successful + When executing query: + """ + INSERT VERTEX test(a) VALUES '102':(3.8); + """ + Then the execution should be successful + When executing query: + """ + INSERT VERTEX test(a) VALUES '103':(-3.2); + """ + Then the execution should be successful + When executing query: + """ + INSERT VERTEX test(a) VALUES '104':(-3.8); + """ + Then the execution should be successful + When executing query: + """ + INSERT VERTEX test(a) VALUES '104':(2147483647.1); + """ + Then an ExecutionError should be raised at runtime: Storage Error: Out of range value. + When executing query: + """ + FETCH PROP ON test '101', '102', '103', '104' YIELD test.a; + """ + Then the result should be, in any order, with relax comparison: + | test.a | + | 3 | + | 4 | + | -3 | + | -4 | + Then drop the used space