Regression test cases:
Compare failed - working_tpch1/misc/bug3669.sql
Compare failed - working_tpch1/misc/MCOL-1246.sql

Since I used MCOL-1559 for the code branch, it's easier to reopen this and close 1673 as the duplicate.

Your comment in MCOL-1673 said maybe undo the change for MCOL-1403 with a caveat for LIKE. Unfortunately, we still have the issue that MariaDB has different behavior for CHAR and VARCHAR. Why this is so is beyond my paygrade. Also, trimming the incoming constant doesn't account for when the column value has trailing spaces. So I came up with another, albeit slightly less efficient, solution that gets (I hope) all the bases.

Build verified: 1.2.4-1 nightly
[dlee@master centos7]$ cat gitversionInfo.txt
server commit:
137b9a8
engine commit:
b3a7559

Reproduced the issue in 1.1.5-1

Verified the fix in 1.2.4-1

MariaDB [mytest]> create table stringtest (c1 char(10), c2 varchar(10), c3 varchar(6))engine=columnstore;
Query OK, 0 rows affected (0.363 sec)

MariaDB [mytest]> insert into stringtest values ('abc','cde','abc'), ('cde','abc','cde');
Query OK, 2 rows affected (0.345 sec)
Records: 2 Duplicates: 0 Warnings: 0

MariaDB [mytest]> select * from stringtest where c1='abc' or c2='abc';
--------------

--------------

--------------
2 rows in set (0.115 sec)

MariaDB [mytest]> select * from stringtest where c1= 'abc ';
--------------

--------------

--------------
1 row in set (0.020 sec)

MariaDB [mytest]> select * from stringtest where c2='abc ';
--------------

--------------

--------------
1 row in set (0.018 sec)

MariaDB [mytest]> select * from stringtest where c1='abc ' or c2='abc ';
--------------

--------------

--------------
2 rows in set (0.020 sec)

MariaDB [mytest]> select * from stringtest where c1 = 'abc' or c3='cde ';
--------------

--------------

--------------
2 rows in set (0.026 sec)

MariaDB [mytest]> select concat('abc ', c2), concat('cde ', c3) from stringtest;
--------------------------------------+

--------------------------------------+

--------------------------------------+
2 rows in set (0.012 sec)

The fix causes a serious performance regression. We need to find a way to do this without string copy.

Added a new fix.

QA: This needs to be checked for performance, as the last fix hurt in this area.

QA:
The compare of strings with trailing blanks is confusing. Given a specific query, you ask, "Should this compare or not?" The answer is, it depends.

If comparing to a constant (the most common of filters), these are the rules:

1. If the column is a VARCHAR or a CHAR, a compare for an operator (=, <, >, etc) should be true if there are trailing spaces in the column data or in the compare data or both.

2. For CHAR and VARCHAR, if the column value has no trailing space, then it should compare false for LIKE if the constant has a trailing space and true if the constant has no trailing space.

3. For CHAR, if the column value has a trailing space, then it should compare false for LIKE if the constant has a trailing space and would otherwise compare (go figure).

4. For VARCHAR, if the column value has a trailing space, then it should compare true for LIKE if the constant has a trailing space and would otherwise compare.

5. For CHAR, if the column value has a trailing space, then it should compare true for LIKE if the constant has no trailing space and otherwise compares.

6. For VARCHAR, if the column value has a trailing space, then it should compare false for LIKE if the constant has no trailing space and otherwise compares.

Truth table for LIKE

Regression test queries/working_tpch1_compareLogOnly/misc/MCOL-1559.sql has been added.

I changed the code to always use boost::regex_match() rather than #ifdefing to regexec(). I'm not sure why the difference was made. Perhaps because of performance (regex == faster?).

Removed a memcpy of the string into a stack allocated array. This seems wasteful, and is surely dangerous if the string was long and blew up the stack. And the code was not portable, using a gcc extension for variable length arrays (VLA). Future versions of gcc will not allow VLA with this syntax. We could do the same thing with alloca, but the copy and potential stack problems say we probably shouldn't.

// Ugh. The strings returned by getStrVal have null padding out to the col width. boost::regex
// considers these nulls significant, but they're not in the pattern, so we need to strip
// them off...

This comment for this bit of code makes no sense. Perhaps in the past there was no version of regexec or regex_match that took a null-terminated string, so it was needed? Basically, the string was copied and then put into a new std::string (making two physical copies), ostensibly with only one NULL.

But we can pass in a NULL terminated string to regexec or to boost::regex_match, so what the what? std::string.c_str() works just fine.

For 1.4, this should all be changed to use std::regex_match (C++11).

Don't forget about performance testing here. Do we want Daniel to do his thing before we merge?

Build tested: 1.2.6-1

engine commit:
d4173ef

Reproduced the issue in bug3669.sql and verified the issue in 1.2.6-1

MariaDB [mytest]> select * from stringtest where c1='abc' or c2='abc';
--------------

--------------

--------------
2 rows in set (0.100 sec)

MariaDB [mytest]> select * from stringtest where c1= 'abc ';
--------------

--------------

--------------
1 row in set (0.026 sec)

MariaDB [mytest]> select * from stringtest where c2='abc ';
--------------

--------------

--------------
1 row in set (0.023 sec)

MariaDB [mytest]> select * from stringtest where c1='abc ' or c2='abc ';
--------------

--------------

--------------
2 rows in set (0.027 sec)

MariaDB [mytest]> select * from stringtest where c1 = 'abc' or c3='cde ';
--------------

--------------

--------------
2 rows in set (0.028 sec)

MariaDB [mytest]> select concat('abc ', c2), concat('cde ', c3) from stringtest;
--------------------------------------+

--------------------------------------+

--------------------------------------+
2 rows in set (0.017 sec)

Did a performance comparison for concat() on integer, date, char-1byte, char-dictionary for 1gb database lineitem table. On average, the fix is about 8% slower.

Opened ticket MCOL-3661 for further performance investigation. Closing this ticket now.

This needs to be reopened. The fix as made has created an unacceptable performance impact.

This was placed in stalled because of some performance concerns. MCOL-3661 addressed those concerns, so this can be closed.