Skip to content

Commit

Permalink
Fix the issue that functions that rely on tipb::FieldType may produ…
Browse files Browse the repository at this point in the history
…ce incorrect results (#8483)

close #8482
  • Loading branch information
SeaRise authored Dec 11, 2023
1 parent 171097e commit 3a72d53
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 4 deletions.
4 changes: 2 additions & 2 deletions dbms/src/Flash/Coprocessor/DAGExpressionAnalyzerHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ String DAGExpressionAnalyzerHelper::buildCastFunctionInternal(
{
static const String tidb_cast_name = "tidb_cast";

String result_name = genFuncString(tidb_cast_name, argument_names, {nullptr});
String result_name = genFuncString(tidb_cast_name, argument_names, {nullptr}, {&field_type});
if (actions->getSampleBlock().has(result_name))
return result_name;

Expand Down Expand Up @@ -299,7 +299,7 @@ String DAGExpressionAnalyzerHelper::buildSingleParamJsonRelatedFunctions(
const auto & input_expr = expr.children(0);
String arg = analyzer->getActions(input_expr, actions);
const auto & collator = getCollatorFromExpr(expr);
String result_name = genFuncString(func_name, {arg}, {collator});
String result_name = genFuncString(func_name, {arg}, {collator}, {&input_expr.field_type(), &expr.field_type()});
if (actions->getSampleBlock().has(result_name))
return result_name;

Expand Down
14 changes: 13 additions & 1 deletion dbms/src/Flash/Coprocessor/DAGUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1405,7 +1405,11 @@ SortDescription getSortDescription(
return order_descr;
}

String genFuncString(const String & func_name, const Names & argument_names, const TiDB::TiDBCollators & collators)
String genFuncString(
const String & func_name,
const Names & argument_names,
const TiDB::TiDBCollators & collators,
const std::vector<const tipb::FieldType *> & field_types)
{
FmtBuffer buf;
buf.fmtAppend("{}({})_collator", func_name, fmt::join(argument_names.begin(), argument_names.end(), ", "));
Expand All @@ -1417,6 +1421,14 @@ String genFuncString(const String & func_name, const Names & argument_names, con
buf.append("_0");
}
buf.append(" ");
buf.joinStr(
field_types.begin(),
field_types.end(),
[](const auto & field_type, FmtBuffer & buffer) {
if likely (field_type)
buffer.fmtAppend("{}|{}", field_type->flag(), field_type->flen());
},
", ");
return buf.toString();
}

Expand Down
6 changes: 5 additions & 1 deletion dbms/src/Flash/Coprocessor/DAGUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ DataTypePtr inferDataType4Literal(const tipb::Expr & expr);
SortDescription getSortDescription(
const std::vector<NameAndTypePair> & order_columns,
const google::protobuf::RepeatedPtrField<tipb::ByItem> & by_items);
String genFuncString(const String & func_name, const Names & argument_names, const TiDB::TiDBCollators & collators);
String genFuncString(
const String & func_name,
const Names & argument_names,
const TiDB::TiDBCollators & collators,
const std::vector<const tipb::FieldType *> & field_types = {});

extern const Int8 VAR_SIZE;

Expand Down
44 changes: 44 additions & 0 deletions tests/fullstack-test/expr/issue_8482.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Copyright 2023 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.

mysql> drop table if exists test.t;
mysql> create table test.t(b json);
mysql> alter table test.t set tiflash replica 1;
mysql> insert into test.t values (true);

func> wait_table test t

mysql> set tidb_allow_mpp=1;set tidb_enforce_mpp=1; set tidb_isolation_read_engines='tiflash'; select b = true from test.t;
+----------+
| b = true |
+----------+
| 0 |
+----------+

mysql> set tidb_allow_mpp=1;set tidb_enforce_mpp=1; set tidb_isolation_read_engines='tiflash'; select b = 1 from test.t;
+-------+
| b = 1 |
+-------+
| 1 |
+-------+

mysql> set tidb_allow_mpp=1;set tidb_enforce_mpp=1; set tidb_isolation_read_engines='tiflash'; select b = true, b = 1 from test.t;
+----------+-------+
| b = true | b = 1 |
+----------+-------+
| 0 | 1 |
+----------+-------+

# Clean up.
mysql> drop table if exists test.t;

0 comments on commit 3a72d53

Please sign in to comment.