fix lpad/rpad trailing zero miss (#9610) (#9630)

close #9465 Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
pingcap · Dec 2, 2024 · a2db730 · a2db730
1 parent e98915d
commit a2db730
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 20 deletions.
diff --git a/dbms/src/Functions/FunctionsString.cpp b/dbms/src/Functions/FunctionsString.cpp
@@ -3203,6 +3203,13 @@ class FunctionTiDBTrim : public IFunction
 
 class TidbPadImpl
 {
+    static void addTrailingZero(ColumnString::Chars_t & res, ColumnString::Offset & res_offset)
+    {
+        res.resize(res.size() + 1);
+        res[res_offset] = '\0';
+        ++res_offset;
+    }
+
 public:
     template <typename IntType, bool IsUTF8, bool IsLeft>
     static void tidbExecutePadImpl(
@@ -3492,9 +3499,7 @@ class TidbPadImpl
             }
             else
             {
-                result_data.resize(result_data.size() + 1);
-                result_data[res_prev_offset] = '\0';
-                res_prev_offset++;
+                addTrailingZero(result_data, res_prev_offset);
             }
 
             string_prev_offset = string_offsets[i];
@@ -3553,9 +3558,7 @@ class TidbPadImpl
             }
             else
             {
-                result_data.resize(result_data.size() + 1);
-                result_data[res_prev_offset] = '\0';
-                res_prev_offset++;
+                addTrailingZero(result_data, res_prev_offset);
             }
 
             string_prev_offset = string_offsets[i];
@@ -3612,9 +3615,7 @@ class TidbPadImpl
             }
             else
             {
-                result_data.resize(result_data.size() + 1);
-                result_data[res_prev_offset] = '\0';
-                res_prev_offset++;
+                addTrailingZero(result_data, res_prev_offset);
             }
 
             padding_prev_offset = (*padding_offsets)[i];
@@ -3670,9 +3671,7 @@ class TidbPadImpl
             }
             else
             {
-                result_data.resize(result_data.size() + 1);
-                result_data[res_prev_offset] = '\0';
-                res_prev_offset++;
+                addTrailingZero(result_data, res_prev_offset);
             }
 
             result_offsets[i] = res_prev_offset;
@@ -3697,6 +3696,7 @@ class TidbPadImpl
 
         if (target_len < 0 || (data_len < static_cast<ColumnString::Offset>(target_len) && pad_len == 0))
         {
+            addTrailingZero(res, res_offset);
             return true;
         }
 
@@ -3748,10 +3748,7 @@ class TidbPadImpl
                 ++left;
             }
         }
-        // Add trailing zero.
-        res.resize(res.size() + 1);
-        res[res_offset] = '\0';
-        res_offset++;
+        addTrailingZero(res, res_offset);
         return false;
     }
 
@@ -3771,6 +3768,7 @@ class TidbPadImpl
 
         if (target_len < 0 || (data_len < static_cast<ColumnString::Offset>(target_len) && pad_len == 0))
         {
+            addTrailingZero(res, res_offset);
             return true;
         }
 
@@ -3823,10 +3821,7 @@ class TidbPadImpl
             copyResult(res, res_offset, data, 0, tmp_target_len);
             res_offset += tmp_target_len;
         }
-        // Add trailing zero.
-        res.resize(res.size() + 1);
-        res[res_offset] = '\0';
-        res_offset++;
+        addTrailingZero(res, res_offset);
         return false;
     }
 

diff --git a/tests/fullstack-test/expr/pad.test b/tests/fullstack-test/expr/pad.test
@@ -111,3 +111,27 @@ mysql> set tidb_isolation_read_engines='tiflash'; set tidb_enforce_mpp=1; select
 
 mysql> set tidb_isolation_read_engines='tiflash'; set tidb_enforce_mpp=1; SELECT max(lpad('y',0,c1)) FROM test.t2
 max(lpad('y',0,c1))
+
+
+mysql> drop table if exists test.t1
+mysql> create table test.t1(c1 varchar(100), c2 int)
+mysql> alter table test.t1 set tiflash replica 1
+mysql> insert into test.t1 values('a', -1)
+func> wait_table test t1
+
+# crc32 will call ColumnString::getDataAt(i), which assume all string rows ends with '\0'.
+mysql> set tidb_isolation_read_engines='tiflash'; set tidb_enforce_mpp=1; select crc32(lpad(c1, c2, 'b')) from test.t1
++--------------------------+
+| crc32(lpad(c1, c2, 'b')) |
++--------------------------+
+|                     NULL |
++--------------------------+
+
+mysql> set tidb_isolation_read_engines='tiflash'; set tidb_enforce_mpp=1; select crc32(rpad(c1, c2, 'b')) from test.t1
++--------------------------+
+| crc32(rpad(c1, c2, 'b')) |
++--------------------------+
+|                     NULL |
++--------------------------+
+
+mysql> drop table if exists test.t1