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

close #9465 Signed-off-by: guo-shaoge <shaoge1994@163.com> Co-authored-by: guo-shaoge <shaoge1994@163.com>
pingcap · Nov 21, 2024 · 99a636e · 99a636e
1 parent d0d987f
commit 99a636e
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
@@ -2964,6 +2964,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(Block & block, const ColumnNumbers & arguments, const size_t result, const String & func_name)
@@ -3143,9 +3150,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];
@@ -3180,9 +3185,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];
@@ -3216,9 +3219,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];
@@ -3251,9 +3252,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;
@@ -3271,6 +3270,7 @@ class TidbPadImpl
 
         if (target_len < 0 || (data_len < static_cast<ColumnString::Offset>(target_len) && pad_len == 0))
         {
+            addTrailingZero(res, res_offset);
             return true;
         }
 
@@ -3322,10 +3322,7 @@ class TidbPadImpl
                 ++left;
             }
         }
-        // Add trailing zero.
-        res.resize(res.size() + 1);
-        res[res_offset] = '\0';
-        res_offset++;
+        addTrailingZero(res, res_offset);
         return false;
     }
 
@@ -3338,6 +3335,7 @@ class TidbPadImpl
 
         if (target_len < 0 || (data_len < static_cast<ColumnString::Offset>(target_len) && pad_len == 0))
         {
+            addTrailingZero(res, res_offset);
             return true;
         }
 
@@ -3390,10 +3388,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