From 5814a80b503262bdb58aff8cf3541c360b4c3f87 Mon Sep 17 00:00:00 2001 From: Leonid Fedorov Date: Thu, 22 May 2025 19:07:45 +0000 Subject: [PATCH] MCOL-4671: MCOL-4622: fix the behavior of both PRs first was playing different with RIGHT and LEFT functions(using the getUintVal and getIntVal accordingly) https://github.com/mariadb-corporation/mariadb-columnstore-engine/pull/3234 second introduced round for ints from double, but added it to uint but not to int missing long doubles as well https://github.com/mariadb-corporation/mariadb-columnstore-engine/pull/3480 --- dbcon/execplan/treenode.h | 8 +-- .../columnstore/bugfixes/mcol_4622.result | 55 +++++++++++++++---- .../columnstore/bugfixes/mcol_4622.test | 32 ++++++----- utils/funcexp/func_left.cpp | 2 +- utils/funcexp/func_right.cpp | 4 +- 5 files changed, 68 insertions(+), 33 deletions(-) diff --git a/dbcon/execplan/treenode.h b/dbcon/execplan/treenode.h index 3fac9428c..a86cb4b07 100644 --- a/dbcon/execplan/treenode.h +++ b/dbcon/execplan/treenode.h @@ -720,12 +720,12 @@ inline int64_t TreeNode::getIntVal() case CalpontSystemCatalog::UINT: return fResult.uintVal; case CalpontSystemCatalog::FLOAT: - case CalpontSystemCatalog::UFLOAT: return (int64_t)fResult.floatVal; + case CalpontSystemCatalog::UFLOAT: return (int64_t)std::llround(fResult.floatVal); case CalpontSystemCatalog::DOUBLE: - case CalpontSystemCatalog::UDOUBLE: return (int64_t)fResult.doubleVal; + case CalpontSystemCatalog::UDOUBLE: return (int64_t)std::llround(fResult.doubleVal); - case CalpontSystemCatalog::LONGDOUBLE: return (int64_t)fResult.longDoubleVal; + case CalpontSystemCatalog::LONGDOUBLE: return (int64_t)std::llround(fResult.longDoubleVal); case CalpontSystemCatalog::DECIMAL: case CalpontSystemCatalog::UDECIMAL: return fResult.decimalVal.toSInt64Round(); @@ -776,7 +776,7 @@ inline uint64_t TreeNode::getUintVal() case CalpontSystemCatalog::DOUBLE: case CalpontSystemCatalog::UDOUBLE: return (uint64_t)std::llround(fResult.doubleVal); - case CalpontSystemCatalog::LONGDOUBLE: return (uint64_t)fResult.longDoubleVal; + case CalpontSystemCatalog::LONGDOUBLE: return (uint64_t)std::llround(fResult.longDoubleVal); case CalpontSystemCatalog::DECIMAL: case CalpontSystemCatalog::UDECIMAL: return fResult.decimalVal.toUInt64Round(); diff --git a/mysql-test/columnstore/bugfixes/mcol_4622.result b/mysql-test/columnstore/bugfixes/mcol_4622.result index 942066127..db02ce1d3 100644 --- a/mysql-test/columnstore/bugfixes/mcol_4622.result +++ b/mysql-test/columnstore/bugfixes/mcol_4622.result @@ -2,16 +2,47 @@ DROP DATABASE IF EXISTS mcol_4622; CREATE DATABASE mcol_4622; USE mcol_4622; DROP TABLE IF EXISTS t1; -CREATE TABLE t1 (a VARCHAR(32), d FLOAT) ENGINE=ColumnStore; -INSERT INTO t1 VALUES ('aaaa', 1.5); -SELECT LEFT(a, d) FROM t1; -LEFT(a, d) -aa -DROP TABLE IF EXISTS t1; -CREATE TABLE t1 (a VARCHAR(32), d DOUBLE) ENGINE=ColumnStore; -INSERT INTO t1 VALUES ('aaaa', 1.5); -SELECT LEFT(a, d) FROM t1; -LEFT(a, d) -aa -DROP TABLE IF EXISTS t1; +CREATE TABLE t1 (a VARCHAR(32), b FLOAT, c DOUBLE) ENGINE=ColumnStore; +INSERT INTO t1 VALUES ('aaaa', 1.5, 1.5); +INSERT INTO t1 VALUES ('abcdefgh', 1.2, 1.2); +INSERT INTO t1 VALUES ('abcdefgh', 1.5, 1.5); +INSERT INTO t1 VALUES ('abcdefgh', 1.7, 1.7); +INSERT INTO t1 VALUES ('abcdefgh', 12, 12); +INSERT INTO t1 VALUES ('abcdefgh', -0, 0); +INSERT INTO t1 VALUES ('abcdefgh', -2, -2); +INSERT INTO t1 VALUES ('abcdefgh', -12, -12); +SELECT LEFT(a, b), LEFT(a, c), RIGHT(a, b), RIGHT(a, c) FROM t1; +LEFT(a, b) LEFT(a, c) RIGHT(a, b) RIGHT(a, c) +aa aa aa aa +a a h h +ab ab gh gh +ab ab gh gh +abcdefgh abcdefgh abcdefgh abcdefgh + + + +create table TFloatsInnoDB(a float, b double) engine innodb; +insert into TFloatsInnoDB values (-3.5,-3.5),(-3.1, -3.1),(-3.0, -3.0),(-2.9,-2.9),(0,0),(2.9, 2.9),(3.1,3.1),(3.5,3.5); +select a, cast(a as signed), cast(a as unsigned), b, cast(b as signed), cast(b as unsigned) from TFloatsInnoDB; +a cast(a as signed) cast(a as unsigned) b cast(b as signed) cast(b as unsigned) +-3.5 -4 0 -3.5 -4 0 +-3.1 -3 0 -3.1 -3 0 +-3 -3 0 -3 -3 0 +-2.9 -3 0 -2.9 -3 0 +0 0 0 0 0 0 +2.9 3 3 2.9 3 3 +3.1 3 3 3.1 3 3 +3.5 4 4 3.5 4 4 +create table TFloatsCS(a float, b double) engine columnstore; +insert into TFloatsCS values (-3.5,-3.5),(-3.1, -3.1),(-3.0, -3.0),(-2.9,-2.9),(0,0),(2.9, 2.9),(3.1,3.1),(3.5,3.5); +select a, cast(a as signed), cast(a as unsigned), b, cast(b as signed), cast(b as unsigned) from TFloatsCS; +a cast(a as signed) cast(a as unsigned) b cast(b as signed) cast(b as unsigned) +-3.5 -4 0 -3.5 -4 0 +-3.1 -3 0 -3.1 -3 0 +-3 -3 0 -3 -3 0 +-2.9 -3 0 -2.9 -3 0 +0 0 0 0 0 0 +2.9 3 3 2.9 3 3 +3.1 3 3 3.1 3 3 +3.5 4 4 3.5 4 4 DROP DATABASE mcol_4622; diff --git a/mysql-test/columnstore/bugfixes/mcol_4622.test b/mysql-test/columnstore/bugfixes/mcol_4622.test index 8d2898d02..862f43239 100644 --- a/mysql-test/columnstore/bugfixes/mcol_4622.test +++ b/mysql-test/columnstore/bugfixes/mcol_4622.test @@ -7,22 +7,26 @@ USE mcol_4622; --disable_warnings DROP TABLE IF EXISTS t1; ---enable_warnings -CREATE TABLE t1 (a VARCHAR(32), d FLOAT) ENGINE=ColumnStore; -INSERT INTO t1 VALUES ('aaaa', 1.5); -SELECT LEFT(a, d) FROM t1; ---disable_warnings -DROP TABLE IF EXISTS t1; ---enable_warnings -CREATE TABLE t1 (a VARCHAR(32), d DOUBLE) ENGINE=ColumnStore; -INSERT INTO t1 VALUES ('aaaa', 1.5); -SELECT LEFT(a, d) FROM t1; +CREATE TABLE t1 (a VARCHAR(32), b FLOAT, c DOUBLE) ENGINE=ColumnStore; +INSERT INTO t1 VALUES ('aaaa', 1.5, 1.5); +INSERT INTO t1 VALUES ('abcdefgh', 1.2, 1.2); +INSERT INTO t1 VALUES ('abcdefgh', 1.5, 1.5); +INSERT INTO t1 VALUES ('abcdefgh', 1.7, 1.7); +INSERT INTO t1 VALUES ('abcdefgh', 12, 12); +INSERT INTO t1 VALUES ('abcdefgh', -0, 0); +INSERT INTO t1 VALUES ('abcdefgh', -2, -2); +INSERT INTO t1 VALUES ('abcdefgh', -12, -12); ---disable_warnings -DROP TABLE IF EXISTS t1; ---enable_warnings +SELECT LEFT(a, b), LEFT(a, c), RIGHT(a, b), RIGHT(a, c) FROM t1; + +create table TFloatsInnoDB(a float, b double) engine innodb; +insert into TFloatsInnoDB values (-3.5,-3.5),(-3.1, -3.1),(-3.0, -3.0),(-2.9,-2.9),(0,0),(2.9, 2.9),(3.1,3.1),(3.5,3.5); +select a, cast(a as signed), cast(a as unsigned), b, cast(b as signed), cast(b as unsigned) from TFloatsInnoDB; + +create table TFloatsCS(a float, b double) engine columnstore; +insert into TFloatsCS values (-3.5,-3.5),(-3.1, -3.1),(-3.0, -3.0),(-2.9,-2.9),(0,0),(2.9, 2.9),(3.1,3.1),(3.5,3.5); +select a, cast(a as signed), cast(a as unsigned), b, cast(b as signed), cast(b as unsigned) from TFloatsCS; ---disable_warnings DROP DATABASE mcol_4622; --enable_warnings diff --git a/utils/funcexp/func_left.cpp b/utils/funcexp/func_left.cpp index 55e42e9c7..93692833b 100644 --- a/utils/funcexp/func_left.cpp +++ b/utils/funcexp/func_left.cpp @@ -60,7 +60,7 @@ std::string Func_left::getStrVal(rowgroup::Row& row, FunctionParm& fp, bool& isN if (isNull || trimLength <= 0) return ""; - size_t trimLengthPositive = static_cast(trimLength); // now we are sure it is positive + size_t trimLengthPositive = trimLength; // now we are sure it is positive size_t charPos; if ((binLen <= trimLengthPositive) || (binLen <= (charPos = cs->charpos(pos, end, trimLengthPositive)))) diff --git a/utils/funcexp/func_right.cpp b/utils/funcexp/func_right.cpp index 4b26265ae..bbec75ebf 100644 --- a/utils/funcexp/func_right.cpp +++ b/utils/funcexp/func_right.cpp @@ -57,11 +57,11 @@ std::string Func_right::getStrVal(rowgroup::Row& row, FunctionParm& fp, bool& is const char* end = pos + binLen; // Negative trim length values are legal, but they don't make any real sense - int64_t trimLength = fp[1]->data()->getUintVal(row, isNull); + int64_t trimLength = fp[1]->data()->getIntVal(row, isNull); if (isNull || trimLength <= 0) return ""; - size_t trimLengthPositive = static_cast(trimLength); // now we are sure it is positive + size_t trimLengthPositive = trimLength; // now we are sure it is positive size_t start = cs->numchars(pos, end); // Here, start is number of characters in src if (start <= trimLengthPositive) return src.safeString("");