ddl: Fix an issue where a query may throw an error during a remote re…

…ad and the precision of a duration data type is changed (release-7.1) (#8812)

close #8601

.clang-format

@@ -66,7 +66,6 @@ ContinuationIndentWidth: 4
 DerivePointerAlignment: false
 DisableFormat:   false
 ForEachMacros:   [ foreach, Q_FOREACH, BOOST_FOREACH ]
-IndentWidth:     4
 IndentWrappedFunctionNames: false
 MacroBlockBegin: ''
 MacroBlockEnd:   ''

dbms/src/DataStreams/NativeBlockInputStream.cpp

@@ -155,9 +155,9 @@ Block NativeBlockInputStream::readImpl()
     }
 
     if (header)
-        CodecUtils::checkColumnSize(header.columns(), columns);
+        CodecUtils::checkColumnSize("NativeBlockInputStream", header.columns(), columns);
     else if (!output_names.empty())
-        CodecUtils::checkColumnSize(output_names.size(), columns);
+        CodecUtils::checkColumnSize("NativeBlockInputStream", output_names.size(), columns);
 
     for (size_t i = 0; i < columns; ++i)
     {
@@ -184,7 +184,7 @@ Block NativeBlockInputStream::readImpl()
         readBinary(type_name, istr);
         if (header)
         {
-            CodecUtils::checkDataTypeName(i, header_datatypes[i].name, type_name);
+            CodecUtils::checkDataTypeName("NativeBlockInputStream", i, header_datatypes[i].name, type_name);
             column.type = header_datatypes[i].type;
         }
         else

dbms/src/Encryption/tests/gtest_rate_limiter.cpp

@@ -70,8 +70,10 @@ TEST(WriteLimiterTest, Rate)
         // make sure that 0.8 * target <= actual_rate <= 1.25 * target
         // hint: the range [0.8, 1.25] is copied from rocksdb,
         // if tests fail, try to enlarge this range.
-        EXPECT_GE(actual_rate / target, 0.80);
-        EXPECT_LE(actual_rate / target, 1.25);
+        EXPECT_GE(actual_rate / target, 0.80)
+            << fmt::format("actual_rate={} target={} elapsed={:.3f}s", actual_rate, target, elapsed);
+        EXPECT_LE(actual_rate / target, 1.30)
+            << fmt::format("actual_rate={} target={} elapsed={:.3f}s", actual_rate, target, elapsed);
     }
 }
 

dbms/src/Flash/Coprocessor/CHBlockChunkCodec.cpp

@@ -190,9 +190,9 @@ void CHBlockChunkCodec::readBlockMeta(ReadBuffer & istr, size_t & columns, size_
     readVarUInt(rows, istr);
 
     if (header)
-        CodecUtils::checkColumnSize(header.columns(), columns);
+        CodecUtils::checkColumnSize("CHBlockChunkCodec", header.columns(), columns);
     else if (!output_names.empty())
-        CodecUtils::checkColumnSize(output_names.size(), columns);
+        CodecUtils::checkColumnSize("CHBlockChunkCodec", output_names.size(), columns);
 }
 
 void CHBlockChunkCodec::readColumnMeta(size_t i, ReadBuffer & istr, ColumnWithTypeAndName & column)
@@ -210,7 +210,7 @@ void CHBlockChunkCodec::readColumnMeta(size_t i, ReadBuffer & istr, ColumnWithTy
     const DataTypeFactory & data_type_factory = DataTypeFactory::instance();
     if (header)
     {
-        CodecUtils::checkDataTypeName(i, header_datatypes[i].name, type_name);
+        CodecUtils::checkDataTypeName("CHBlockChunkCodec", i, header_datatypes[i].name, type_name);
         column.type = header_datatypes[i].type;
     }
     else

dbms/src/Flash/Coprocessor/CHBlockChunkCodecV1.cpp

@@ -63,7 +63,7 @@ Block DecodeHeader(ReadBuffer & istr, const Block & header, size_t & total_rows)
         readVarUInt(total_rows, istr);
     }
     if (header)
-        CodecUtils::checkColumnSize(header.columns(), columns);
+        CodecUtils::checkColumnSize("CHBlockChunkCodecV1", header.columns(), columns);
 
     for (size_t i = 0; i < columns; ++i)
     {
@@ -76,7 +76,11 @@ Block DecodeHeader(ReadBuffer & istr, const Block & header, size_t & total_rows)
             readBinary(type_name, istr);
             if (header)
             {
-                CodecUtils::checkDataTypeName(i, header.getByPosition(i).type->getName(), type_name);
+                CodecUtils::checkDataTypeName(
+                    "CHBlockChunkCodecV1",
+                    i,
+                    header.getByPosition(i).type->getName(),
+                    type_name);
                 column.type = header.getByPosition(i).type;
             }
             else
@@ -459,11 +463,15 @@ CHBlockChunkCodecV1::CHBlockChunkCodecV1(const Block & header_)
 
 static void checkSchema(const Block & header, const Block & block)
 {
-    CodecUtils::checkColumnSize(header.columns(), block.columns());
+    CodecUtils::checkColumnSize("CHBlockChunkCodecV1", header.columns(), block.columns());
     for (size_t column_index = 0; column_index < header.columns(); ++column_index)
     {
         auto && type_name = block.getByPosition(column_index).type->getName();
-        CodecUtils::checkDataTypeName(column_index, header.getByPosition(column_index).type->getName(), type_name);
+        CodecUtils::checkDataTypeName(
+            "CHBlockChunkCodecV1",
+            column_index,
+            header.getByPosition(column_index).type->getName(),
+            type_name);
     }
 }
 

dbms/src/Flash/Coprocessor/CodecUtils.cpp

@@ -25,20 +25,21 @@ extern const int LOGICAL_ERROR;
 
 namespace CodecUtils
 {
-void checkColumnSize(size_t expected, size_t actual)
+void checkColumnSize(const String & identifier, size_t expected, size_t actual)
 {
     if unlikely (expected != actual)
         throw Exception(
-            fmt::format("NativeBlockInputStream schema mismatch, expected {}, actual {}.", expected, actual),
+            fmt::format("{} schema size mismatch, expected {}, actual {}.", identifier, expected, actual),
             ErrorCodes::LOGICAL_ERROR);
 }
 
-void checkDataTypeName(size_t column_index, const String & expected, const String & actual)
+void checkDataTypeName(const String & identifier, size_t column_index, const String & expected, const String & actual)
 {
     if unlikely (expected != actual)
         throw Exception(
             fmt::format(
-                "NativeBlockInputStream schema mismatch at column {}, expected {}, actual {}",
+                "{} schema mismatch at column {}, expected {}, actual {}",
+                identifier,
                 column_index,
                 expected,
                 actual),

dbms/src/Flash/Coprocessor/CodecUtils.h

@@ -32,7 +32,7 @@ struct DataTypeWithTypeName
     String name;
 };
 
-void checkColumnSize(size_t expected, size_t actual);
-void checkDataTypeName(size_t column_index, const String & expected, const String & actual);
+void checkColumnSize(const String & identifier, size_t expected, size_t actual);
+void checkDataTypeName(const String & identifier, size_t column_index, const String & expected, const String & actual);
 } // namespace CodecUtils
-} // namespace DB
+} // namespace DB

dbms/src/TiDB/Schema/SchemaBuilder.cpp

@@ -142,9 +142,9 @@ bool typeDiffers(const TiDB::ColumnInfo & a, const TiDB::ColumnInfo & b)
     {
         return a.flen != b.flen || a.decimal != b.decimal;
     }
-    else if (a.tp == TiDB::TypeDatetime || a.tp == TiDB::TypeDate || a.tp == TiDB::TypeTimestamp)
+    else if (a.tp == TiDB::TypeDatetime || a.tp == TiDB::TypeDate || a.tp == TiDB::TypeTimestamp || a.tp == TiDB::TypeTime)
     {
-        // detect fsp changed in MyDateTime/MyTime
+        // detect fsp changed in MyDateTime/MyTime/MyDuration (`Datetime`/`TIMESTAMP`/`TIME` in TiDB)
         return a.flen != b.flen || a.decimal != b.decimal;
     }
     return false;

tests/_env.sh

@@ -42,7 +42,7 @@ export storage_db="system"
 export tidb_server="127.0.0.1"
 
 # TiDB port
-export tidb_port="4000"
+export tidb_port="${tidb_port:-4000}"
 
 # TiDB status port
 export tidb_status_port="10080"
@@ -54,8 +54,7 @@ export tidb_db="test"
 export tidb_table="t"
 
 # Whether run scripts with verbose output
-export verbose="false"
-# export verbose="true"
+export verbose="${verbose:-"false"}"
 
 # Setup running env vars
 #source ../../_vars.sh

tests/fullstack-test/issues/issue_8601.test

@@ -0,0 +1,72 @@
+# Copyright 2024 PingCAP, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Preparation.
+=> DBGInvoke __init_fail_point()
+=> DBGInvoke __enable_schema_sync_service('false')
+
+mysql> drop table if exists test.t;
+mysql> create table if not exists test.t(a time(4));
+
+mysql> insert into test.t values('700:10:10.123456');
+mysql> insert into test.t values('-700:10:10.123456');
+mysql> alter table test.t set tiflash replica 1;
+func> wait_table test t
+
+## time(4) to time(6)
+mysql> alter table test.t modify column a time(6);
+
+mysql> use test; set tidb_enforce_mpp=1; set tidb_isolation_read_engines='tiflash'; select a from test.t;
++-------------------+
+| a                 |
++-------------------+
+| 700:10:10.123500  |
+| -700:10:10.123500 |
++-------------------+
+
+=> DBGInvoke __enable_fail_point(force_remote_read_for_batch_cop)
+mysql> use test; set tidb_enforce_mpp=1; set tidb_isolation_read_engines='tiflash'; select a from test.t;
++-------------------+
+| a                 |
++-------------------+
+| 700:10:10.123500  |
+| -700:10:10.123500 |
++-------------------+
+=> DBGInvoke __disable_fail_point(force_remote_read_for_batch_cop)
+
+## time(6) to time(2)
+mysql> alter table test.t modify column a time(2);
+
+mysql> use test; set tidb_enforce_mpp=1; set tidb_isolation_read_engines='tiflash'; select a from test.t;
++---------------+
+| a             |
++---------------+
+| 700:10:10.12  |
+| -700:10:10.12 |
++---------------+
+
+=> DBGInvoke __enable_fail_point(force_remote_read_for_batch_cop)
+mysql> use test; set tidb_enforce_mpp=1; set tidb_isolation_read_engines='tiflash'; select a from test.t;
++---------------+
+| a             |
++---------------+
+| 700:10:10.12  |
+| -700:10:10.12 |
++---------------+
+=> DBGInvoke __disable_fail_point(force_remote_read_for_batch_cop)
+
+# Clean up.
+mysql> drop table if exists test.t
+
+=> DBGInvoke __enable_schema_sync_service('true')