From c16b0f6ad7e5bd4ba19c66bf5317ebbecc45e596 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Tue, 3 Aug 2021 12:53:05 +0400 Subject: [PATCH] MCOL-4823 WHERE char_colleftColType = leftColType; filterCmd->rightColType = rightColType; which exist in the parent's FilterCommand::duplicate(). Rewriting the code to avoid duplication by using more inherited methods/constructors. This reduces the probability of similar bugs in the future. --- .../columnstore/basic/r/type_string.result | 80 +++++++++++++++++++ .../columnstore/basic/t/type_string.test | 67 ++++++++++++++++ primitives/primproc/columncommand.cpp | 2 +- primitives/primproc/command.cpp | 11 --- primitives/primproc/command.h | 8 +- primitives/primproc/dictstep.cpp | 2 +- primitives/primproc/filtercommand.cpp | 22 +---- primitives/primproc/filtercommand.h | 10 ++- primitives/primproc/passthrucommand.cpp | 2 +- primitives/primproc/rtscommand.cpp | 2 +- 10 files changed, 161 insertions(+), 45 deletions(-) create mode 100644 mysql-test/columnstore/basic/r/type_string.result create mode 100644 mysql-test/columnstore/basic/t/type_string.test diff --git a/mysql-test/columnstore/basic/r/type_string.result b/mysql-test/columnstore/basic/r/type_string.result new file mode 100644 index 000000000..06cdc3bd4 --- /dev/null +++ b/mysql-test/columnstore/basic/r/type_string.result @@ -0,0 +1,80 @@ +DROP DATABASE IF EXISTS mcs_type_string; +CREATE DATABASE mcs_type_string; +# +# MCOL-4823 WHERE char_col'; +DECLARE onerow TEXT DEFAULT '(''a'',''aaaa'',''AAAA'')'; +DECLARE rows256 TEXT DEFAULT CONCAT(REPEAT(CONCAT(onerow, ','), 255), onerow); +-- The problem was repeatable with at least 122881 records +-- For faster data loading, insert 256 records at a time 480 times +-- That gives 480*256=122880 records +FOR i IN 1..480 +DO +EXECUTE IMMEDIATE REPLACE(ins, '', rows256); +END FOR; +-- Now insert one more record to make 122881 records +EXECUTE IMMEDIATE REPLACE(ins, '', onerow); +END +$$ +SELECT count(*) FROM t1 WHERE l_comment < l_shipinstruct; +count(*) +122881 +SELECT count(*) FROM t1 WHERE l_comment <= l_shipinstruct; +count(*) +122881 +SELECT count(*) FROM t1 WHERE l_comment >= l_shipinstruct; +count(*) +0 +SELECT count(*) FROM t1 WHERE l_comment > l_shipinstruct; +count(*) +0 +SELECT count(*) FROM t1 WHERE l_shipinstruct < l_comment; +count(*) +0 +SELECT count(*) FROM t1 WHERE l_shipinstruct <= l_comment; +count(*) +0 +SELECT count(*) FROM t1 WHERE l_shipinstruct >= l_comment; +count(*) +122881 +SELECT count(*) FROM t1 WHERE l_shipinstruct > l_comment; +count(*) +122881 +CREATE TABLE t2 LIKE t1; +ALTER TABLE t2 ENGINE=ColumnStore; +INSERT INTO t2 SELECT * FROM t1; +SELECT count(*) FROM t2 WHERE l_comment < l_shipinstruct; +count(*) +122881 +SELECT count(*) FROM t2 WHERE l_comment <= l_shipinstruct; +count(*) +122881 +SELECT count(*) FROM t2 WHERE l_comment >= l_shipinstruct; +count(*) +0 +SELECT count(*) FROM t2 WHERE l_comment > l_shipinstruct; +count(*) +0 +SELECT count(*) FROM t2 WHERE l_shipinstruct < l_comment; +count(*) +0 +SELECT count(*) FROM t2 WHERE l_shipinstruct <= l_comment; +count(*) +0 +SELECT count(*) FROM t2 WHERE l_shipinstruct >= l_comment; +count(*) +122881 +SELECT count(*) FROM t2 WHERE l_shipinstruct > l_comment; +count(*) +122881 +DROP TABLE t2; +DROP TABLE t1; +DROP DATABASE mcs_type_string; diff --git a/mysql-test/columnstore/basic/t/type_string.test b/mysql-test/columnstore/basic/t/type_string.test new file mode 100644 index 000000000..baf37e627 --- /dev/null +++ b/mysql-test/columnstore/basic/t/type_string.test @@ -0,0 +1,67 @@ +--source ../include/have_columnstore.inc + +--disable_warnings +DROP DATABASE IF EXISTS mcs_type_string; +--enable_warnings +CREATE DATABASE mcs_type_string; + + +--echo # +--echo # MCOL-4823 WHERE char_col'; + DECLARE onerow TEXT DEFAULT '(''a'',''aaaa'',''AAAA'')'; + DECLARE rows256 TEXT DEFAULT CONCAT(REPEAT(CONCAT(onerow, ','), 255), onerow); + + -- The problem was repeatable with at least 122881 records + -- For faster data loading, insert 256 records at a time 480 times + -- That gives 480*256=122880 records + FOR i IN 1..480 + DO + EXECUTE IMMEDIATE REPLACE(ins, '', rows256); + END FOR; + + -- Now insert one more record to make 122881 records + EXECUTE IMMEDIATE REPLACE(ins, '', onerow); +END +$$ +DELIMITER ;$$ + +SELECT count(*) FROM t1 WHERE l_comment < l_shipinstruct; +SELECT count(*) FROM t1 WHERE l_comment <= l_shipinstruct; +SELECT count(*) FROM t1 WHERE l_comment >= l_shipinstruct; +SELECT count(*) FROM t1 WHERE l_comment > l_shipinstruct; +SELECT count(*) FROM t1 WHERE l_shipinstruct < l_comment; +SELECT count(*) FROM t1 WHERE l_shipinstruct <= l_comment; +SELECT count(*) FROM t1 WHERE l_shipinstruct >= l_comment; +SELECT count(*) FROM t1 WHERE l_shipinstruct > l_comment; + +CREATE TABLE t2 LIKE t1; +ALTER TABLE t2 ENGINE=ColumnStore; +INSERT INTO t2 SELECT * FROM t1; + +SELECT count(*) FROM t2 WHERE l_comment < l_shipinstruct; +SELECT count(*) FROM t2 WHERE l_comment <= l_shipinstruct; +SELECT count(*) FROM t2 WHERE l_comment >= l_shipinstruct; +SELECT count(*) FROM t2 WHERE l_comment > l_shipinstruct; +SELECT count(*) FROM t2 WHERE l_shipinstruct < l_comment; +SELECT count(*) FROM t2 WHERE l_shipinstruct <= l_comment; +SELECT count(*) FROM t2 WHERE l_shipinstruct >= l_comment; +SELECT count(*) FROM t2 WHERE l_shipinstruct > l_comment; + + +DROP TABLE t2; +DROP TABLE t1; + + +DROP DATABASE mcs_type_string; diff --git a/primitives/primproc/columncommand.cpp b/primitives/primproc/columncommand.cpp index 63f8b1604..9635e6017 100644 --- a/primitives/primproc/columncommand.cpp +++ b/primitives/primproc/columncommand.cpp @@ -893,7 +893,7 @@ void ColumnCommand::duplicate(ColumnCommand* cc) cc->lastLbid = lastLbid; cc->r = r; cc->rowSize = rowSize; - cc->Command::duplicate(this); + cc->Command::operator=(*this); } SCommand ColumnCommand::duplicate() diff --git a/primitives/primproc/command.cpp b/primitives/primproc/command.cpp index 4e3ccaab1..54432d277 100644 --- a/primitives/primproc/command.cpp +++ b/primitives/primproc/command.cpp @@ -161,15 +161,4 @@ bool Command::operator==(const Command& c) const return true; } -void Command::duplicate(Command* c) -{ - bpp = c->bpp; - cmdType = c->cmdType; - fFilterFeeder = c->fFilterFeeder; - OID = c->OID; - tupleKey = c->tupleKey; - queryUuid = c->queryUuid; - stepUuid = c->stepUuid; -} - } diff --git a/primitives/primproc/command.h b/primitives/primproc/command.h index 98485e45c..64a77e0c5 100644 --- a/primitives/primproc/command.h +++ b/primitives/primproc/command.h @@ -117,12 +117,8 @@ protected: boost::uuids::uuid queryUuid; boost::uuids::uuid stepUuid; - void duplicate(Command*); - -private: - Command(); - Command(const Command&); - + Command(const Command &rhs) = default; + Command & operator=(const Command & rhs) = default; }; } diff --git a/primitives/primproc/dictstep.cpp b/primitives/primproc/dictstep.cpp index 2981e4ff8..52c012df7 100644 --- a/primitives/primproc/dictstep.cpp +++ b/primitives/primproc/dictstep.cpp @@ -697,7 +697,7 @@ SCommand DictStep::duplicate() ds->filterString = filterString; ds->filterCount = filterCount; ds->charsetNumber = charsetNumber; - ds->Command::duplicate(this); + ds->Command::operator=(*this); return ret; } diff --git a/primitives/primproc/filtercommand.cpp b/primitives/primproc/filtercommand.cpp index f63cb4beb..88ecb03e7 100644 --- a/primitives/primproc/filtercommand.cpp +++ b/primitives/primproc/filtercommand.cpp @@ -233,16 +233,7 @@ void FilterCommand::nextLBID() SCommand FilterCommand::duplicate() { - SCommand ret; - FilterCommand* filterCmd; - - ret.reset(new FilterCommand()); - filterCmd = (FilterCommand*) ret.get(); - filterCmd->fBOP = fBOP; - filterCmd->leftColType = leftColType; - filterCmd->rightColType = rightColType; - filterCmd->Command::duplicate(this); - return ret; + return SCommand(new FilterCommand(*this)); } @@ -516,16 +507,7 @@ StrFilterCmd::~StrFilterCmd() SCommand StrFilterCmd::duplicate() { - SCommand ret; - StrFilterCmd* filterCmd; - - ret.reset(new StrFilterCmd()); - filterCmd = (StrFilterCmd*) ret.get(); - filterCmd->fBOP = fBOP; - filterCmd->fCompare = fCompare; - filterCmd->fCharLength = fCharLength; - - return ret; + return SCommand(new StrFilterCmd(*this)); } diff --git a/primitives/primproc/filtercommand.h b/primitives/primproc/filtercommand.h index 263523bef..791c35c3e 100644 --- a/primitives/primproc/filtercommand.h +++ b/primitives/primproc/filtercommand.h @@ -88,9 +88,10 @@ protected: execplan::CalpontSystemCatalog::ColType leftColType; execplan::CalpontSystemCatalog::ColType rightColType; + FilterCommand(const FilterCommand&rhs) = default; + private: - // disabled copy constructor and operator - FilterCommand(const FilterCommand&); + // disabled copy operator FilterCommand& operator=(const FilterCommand&); }; @@ -156,9 +157,10 @@ protected: // colWidth of columns the don't need a dictionary size_t fCharLength; + StrFilterCmd(const StrFilterCmd &rhs) = default; + private: - // disabled copy constructor and operator - StrFilterCmd(const StrFilterCmd&); + // disabled copy operator StrFilterCmd& operator=(const StrFilterCmd&); }; diff --git a/primitives/primproc/passthrucommand.cpp b/primitives/primproc/passthrucommand.cpp index 7f345d6d8..5062ce761 100644 --- a/primitives/primproc/passthrucommand.cpp +++ b/primitives/primproc/passthrucommand.cpp @@ -191,7 +191,7 @@ SCommand PassThruCommand::duplicate() ret.reset(new PassThruCommand()); p = (PassThruCommand*) ret.get(); p->colWidth = colWidth; - p->Command::duplicate(this); + p->Command::operator=(*this); return ret; } diff --git a/primitives/primproc/rtscommand.cpp b/primitives/primproc/rtscommand.cpp index 582021ec8..c4d310539 100644 --- a/primitives/primproc/rtscommand.cpp +++ b/primitives/primproc/rtscommand.cpp @@ -205,7 +205,7 @@ SCommand RTSCommand::duplicate() rts->col.reset(ColumnCommandFabric::duplicate(col)); rts->dict = dict; - rts->Command::duplicate(this); + rts->Command::operator=(*this); return ret; }