MCOL-4823 WHERE char_col<varchar_col returns a wrong result of a large table (#2060)

SCommand StrFilterCmd::duplicate() missed these two lines:

    filterCmd->leftColType = 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.

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<varchar_col returns a wrong result of a large table
+#
+CREATE TABLE t1
+(
+l_returnflag char(1) CHARACTER SET latin1 COLLATE latin1_bin,
+l_shipinstruct char(25) CHARACTER SET latin1 COLLATE latin1_bin,
+l_comment varchar(44) CHARACTER SET latin1 COLLATE latin1_bin
+) ENGINE=MyISAM;
+BEGIN NOT ATOMIC
+DECLARE ins TEXT DEFAULT 'INSERT INTO t1 VALUES <rows>';
+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, '<rows>', rows256);
+END FOR;
+-- Now insert one more record to make 122881 records
+EXECUTE IMMEDIATE REPLACE(ins, '<rows>', 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;

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<varchar_col returns a wrong result of a large table
+--echo #
+
+CREATE TABLE t1
+(
+  l_returnflag char(1) CHARACTER SET latin1 COLLATE latin1_bin,
+  l_shipinstruct char(25) CHARACTER SET latin1 COLLATE latin1_bin,
+  l_comment varchar(44) CHARACTER SET latin1 COLLATE latin1_bin
+) ENGINE=MyISAM;
+
+DELIMITER $$;
+BEGIN NOT ATOMIC
+  DECLARE ins TEXT DEFAULT 'INSERT INTO t1 VALUES <rows>';
+  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, '<rows>', rows256);
+  END FOR;
+
+  -- Now insert one more record to make 122881 records
+  EXECUTE IMMEDIATE REPLACE(ins, '<rows>', 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;

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()

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;
-}
-
 }

primitives/primproc/command.h

@@ -117,12 +117,8 @@ protected:
     boost::uuids::uuid queryUuid;
     boost::uuids::uuid stepUuid;
 
-    void duplicate(Command*);
+    Command(const Command &rhs) = default;
-
+    Command & operator=(const Command & rhs) = default;
-private:
-    Command();
-    Command(const Command&);
-
 };
 
 }

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;
 }
 

primitives/primproc/filtercommand.cpp

@@ -233,16 +233,7 @@ void FilterCommand::nextLBID()
 
 SCommand FilterCommand::duplicate()
 {
-    SCommand ret;
+    return SCommand(new FilterCommand(*this));
-    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;
 }
 
 
@@ -516,16 +507,7 @@ StrFilterCmd::~StrFilterCmd()
 
 SCommand StrFilterCmd::duplicate()
 {
-    SCommand ret;
+    return SCommand(new StrFilterCmd(*this));
-    StrFilterCmd* filterCmd;
-
-    ret.reset(new StrFilterCmd());
-    filterCmd = (StrFilterCmd*) ret.get();
-    filterCmd->fBOP = fBOP;
-    filterCmd->fCompare = fCompare;
-    filterCmd->fCharLength = fCharLength;
-
-    return ret;
 }
 
 

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
+    // disabled copy operator
-    FilterCommand(const FilterCommand&);
     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
+    // disabled copy operator
-    StrFilterCmd(const StrFilterCmd&);
     StrFilterCmd& operator=(const StrFilterCmd&);
 };
 

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;
 }
 

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;
 }