From 57e2375dbc2260ddca472c80b268436b83b46a7f Mon Sep 17 00:00:00 2001 From: Alexander Presnyakov Date: Wed, 3 Jul 2024 19:11:19 +0400 Subject: [PATCH] fix(funcexp): MCOL-4671 Fix behaviour of LEFT/RIGHT functions when negative trim length value is passedB --- .../mcol-4671-func_left_right_negative.result | 44 +++++++++++++++++++ .../mcol-4671-func_left_right_negative.test | 44 +++++++++++++++++++ utils/funcexp/func_left.cpp | 6 ++- utils/funcexp/func_right.cpp | 8 ++-- 4 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 mysql-test/columnstore/bugfixes/mcol-4671-func_left_right_negative.result create mode 100644 mysql-test/columnstore/bugfixes/mcol-4671-func_left_right_negative.test diff --git a/mysql-test/columnstore/bugfixes/mcol-4671-func_left_right_negative.result b/mysql-test/columnstore/bugfixes/mcol-4671-func_left_right_negative.result new file mode 100644 index 000000000..6c6f7edca --- /dev/null +++ b/mysql-test/columnstore/bugfixes/mcol-4671-func_left_right_negative.result @@ -0,0 +1,44 @@ +DROP DATABASE IF EXISTS `mcol 4671`; +CREATE DATABASE `mcol 4671`; +USE `mcol 4671`; +CREATE TABLE t_tinyint (a TINYINT) ENGINE=ColumnStore; +INSERT INTO t_tinyint VALUES (-1); +SELECT LEFT('abc',a) FROM t_tinyint; +LEFT('abc',a) + +SELECT RIGHT('abc',a) FROM t_tinyint; +RIGHT('abc',a) + +CREATE TABLE t_smallint (a SMALLINT) ENGINE=ColumnStore; +INSERT INTO t_smallint VALUES (-1); +SELECT LEFT('abc',a) FROM t_smallint; +LEFT('abc',a) + +SELECT RIGHT('abc',a) FROM t_smallint; +RIGHT('abc',a) + +CREATE TABLE t_mediumint (a MEDIUMINT) ENGINE=ColumnStore; +INSERT INTO t_mediumint VALUES (-1); +SELECT LEFT('abc',a) FROM t_mediumint; +LEFT('abc',a) + +SELECT RIGHT('abc',a) FROM t_mediumint; +RIGHT('abc',a) + +CREATE TABLE t_int (a INT) ENGINE=ColumnStore; +INSERT INTO t_int VALUES (-1); +SELECT LEFT('abc',a) FROM t_int; +LEFT('abc',a) + +SELECT RIGHT('abc',a) FROM t_int; +RIGHT('abc',a) + +CREATE TABLE t_bigint (a BIGINT) ENGINE=ColumnStore; +INSERT INTO t_bigint VALUES (-1); +SELECT LEFT('abc',a) FROM t_bigint; +LEFT('abc',a) + +SELECT RIGHT('abc',a) FROM t_bigint; +RIGHT('abc',a) + +DROP DATABASE `mcol 4671`; diff --git a/mysql-test/columnstore/bugfixes/mcol-4671-func_left_right_negative.test b/mysql-test/columnstore/bugfixes/mcol-4671-func_left_right_negative.test new file mode 100644 index 000000000..2a58e6505 --- /dev/null +++ b/mysql-test/columnstore/bugfixes/mcol-4671-func_left_right_negative.test @@ -0,0 +1,44 @@ +--source ../include/have_columnstore.inc +--disable_warnings +DROP DATABASE IF EXISTS `mcol 4671`; +--enable_warnings +CREATE DATABASE `mcol 4671`; +USE `mcol 4671`; + +# tinyint +CREATE TABLE t_tinyint (a TINYINT) ENGINE=ColumnStore; +INSERT INTO t_tinyint VALUES (-1); +SELECT LEFT('abc',a) FROM t_tinyint; + +SELECT RIGHT('abc',a) FROM t_tinyint; + +# smallint +CREATE TABLE t_smallint (a SMALLINT) ENGINE=ColumnStore; +INSERT INTO t_smallint VALUES (-1); +SELECT LEFT('abc',a) FROM t_smallint; + +SELECT RIGHT('abc',a) FROM t_smallint; + +# mediumint +CREATE TABLE t_mediumint (a MEDIUMINT) ENGINE=ColumnStore; +INSERT INTO t_mediumint VALUES (-1); +SELECT LEFT('abc',a) FROM t_mediumint; + +SELECT RIGHT('abc',a) FROM t_mediumint; + +# int +CREATE TABLE t_int (a INT) ENGINE=ColumnStore; +INSERT INTO t_int VALUES (-1); +SELECT LEFT('abc',a) FROM t_int; + +SELECT RIGHT('abc',a) FROM t_int; + +# bigint +CREATE TABLE t_bigint (a BIGINT) ENGINE=ColumnStore; +INSERT INTO t_bigint VALUES (-1); +SELECT LEFT('abc',a) FROM t_bigint; + +SELECT RIGHT('abc',a) FROM t_bigint; + +# cleanup +DROP DATABASE `mcol 4671`; diff --git a/utils/funcexp/func_left.cpp b/utils/funcexp/func_left.cpp index eefdeecfd..3a182192b 100644 --- a/utils/funcexp/func_left.cpp +++ b/utils/funcexp/func_left.cpp @@ -55,13 +55,15 @@ std::string Func_left::getStrVal(rowgroup::Row& row, FunctionParm& fp, bool& isN const char* pos = src.str(); const char* end = pos + binLen; - size_t trimLength = fp[1]->data()->getUintVal(row, isNull); + // Negative trim length values are legal, but they don't make any real sense + 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 charPos; - if ((binLen <= trimLength) || (binLen <= (charPos = cs->charpos(pos, end, trimLength)))) + if ((binLen <= trimLengthPositive) || (binLen <= (charPos = cs->charpos(pos, end, trimLengthPositive)))) { return src.safeString(""); } diff --git a/utils/funcexp/func_right.cpp b/utils/funcexp/func_right.cpp index 6a1a7d0e2..3cdb746b3 100644 --- a/utils/funcexp/func_right.cpp +++ b/utils/funcexp/func_right.cpp @@ -56,15 +56,17 @@ std::string Func_right::getStrVal(rowgroup::Row& row, FunctionParm& fp, bool& is const char* pos = src.str(); const char* end = pos + binLen; - size_t trimLength = fp[1]->data()->getUintVal(row, isNull); + // Negative trim length values are legal, but they don't make any real sense + int64_t trimLength = fp[1]->data()->getUintVal(row, isNull); if (isNull || trimLength <= 0) return ""; + size_t trimLengthPositive = static_cast(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 <= trimLength) + if (start <= trimLengthPositive) return src.safeString(""); start = cs->charpos(pos, end, - start - trimLength); // Here, start becomes number of bytes into src to start copying + start - trimLengthPositive); // Here, start becomes number of bytes into src to start copying std::string ret(pos + start, binLen - start); return ret;